bugAVR C Runtime Library - Bugs: bug #43828, wdt.h: Wrong inline assembler...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #43828: wdt.h: Wrong inline assembler arguments

Submitter:  Georg-Johann Lay <gjlayde>
Submitted:  Mon 15 Dec 2014 07:25:34 PM UTC
   
 
Category:  Header Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Header files
Status:  Fixed Assigned to:  pitchumani
Percent Complete:  0% Open/Closed:  Closed
Release:  1.8.0 Fixed Release:  None

Thu 18 Jun 2015 09:46:25 AM UTC, comment #2: 
Pitchumani <pitchumani>
Group Member
Wed 24 Dec 2014 06:40:17 AM UTC, comment #1: 

Below is the patch to fix this issue.

diff --git a/avr-libc/include/avr/wdt.h b/avr-libc/include/avr/wdt.h
index fc15885..3130e67 100644
--- a/avr-libc/include/avr/wdt.h
+++ b/avr-libc/include/avr/wdt.h
@@ -148,25 +148,24 @@
 */
 #define wdt_enable(timeout) \
 do { \
-uint8_t temp = 0; \
+uint8_t temp; \
 __asm__ _volatile_ (         \
     "in _tmp_reg_, %[rampd]"              "\n\t" \
     "out %[rampd], _zero_reg_"            "\n\t" \
     "out %[ccp_reg], %[ioreg_cen_mask]"     "\n\t" \
     "sts %[wdt_reg], %[wdt_enable_timeout]" "\n\t" \
     "1:lds %[tmp], %[wdt_status_reg]"       "\n\t" \
-    "sbrc  %[tmp], %[wdt_syncbusy_bit]"     "\n\t" \
+    "sbrc %[tmp], %[wdt_syncbusy_bit]"     "\n\t" \
     "rjmp 1b"                               "\n\t" \
     "out %[rampd], _tmp_reg_"             "\n\t" \
-    : "=r" (temp) \
+    : [tmp]                "=r" (temp) \
     : [rampd]              "M" (_SFR_MEM_ADDR(RAMPD)),      \
       [ccp_reg]            "I" (_SFR_MEM_ADDR(CCP)),        \
       [ioreg_cen_mask]     "r" ((uint8_t)CCP_IOREG_gc),     \
       [wdt_reg]            "M" (_SFR_MEM_ADDR(WDT_CTRL)),   \
       [wdt_enable_timeout] "r" ((uint8_t)(WDT_CEN_bm | WDT_ENABLE_bm | timeout)), \
       [wdt_status_reg]     "M" (_SFR_MEM_ADDR(WDT_STATUS)), \
-      [wdt_syncbusy_bit]   "I" (WDT_SYNCBUSY_bm),           \
-      [tmp]                "r" (temp)                       \
+      [wdt_syncbusy_bit]   "I" (WDT_SYNCBUSY_bm)            \
     : "r0" \
 ); \
 } while(0)

Pitchumani <pitchumani>
Group Member
Mon 15 Dec 2014 07:25:34 PM UTC, original submission:  

wdt.h comes with wrong inline assembler arguments.  Let me give an example (2014-12-15 SVN trunk r2641):



#define wdt_enable(timeout) \
do { \
uint8_t temp = 0; \
__asm__ __volatile__ (         \
    "in __tmp_reg__, %[rampd]"              "\n\t" \
    "out %[rampd], __zero_reg__"            "\n\t" \
    "out %[ccp_reg], %[ioreg_cen_mask]"     "\n\t" \
    "sts %[wdt_reg], %[wdt_enable_timeout]" "\n\t" \
    "1:lds %[tmp], %[wdt_status_reg]"       "\n\t" \
    "sbrc  %[tmp], %[wdt_syncbusy_bit]"     "\n\t" \
    "rjmp 1b"                               "\n\t" \
    "out %[rampd], __tmp_reg__"             "\n\t" \
    : "=r" (temp) \
    : [rampd]              "M" (_SFR_MEM_ADDR(RAMPD)),      \
      [ccp_reg]            "I" (_SFR_MEM_ADDR(CCP)),        \
      [ioreg_cen_mask]     "r" ((uint8_t)CCP_IOREG_gc),     \
      [wdt_reg]            "M" (_SFR_MEM_ADDR(WDT_CTRL)),   \
      [wdt_enable_timeout] "r" ((uint8_t)(WDT_CEN_bm | WDT_ENABLE_bm | timeout)), \
      [wdt_status_reg]     "M" (_SFR_MEM_ADDR(WDT_STATUS)), \
      [wdt_syncbusy_bit]   "I" (WDT_SYNCBUSY_bm),           \
      [tmp]                "r" (temp)                       \
    : "r0" \
); \
} while(0)


Operand %[tmp] (%8) is changed but not indicated as so by means of constraint modifiers.

Notice that the compiler may allocate different registers to %0 and %8.  Hence, the fix is to refer to %0 instead of to %[tmp] resp. moving %[tmp] to the first operand.  Also notice that %8 is unused and can be dropped.  temp is loaded with 0 but respective content of %[tmp] is discarded by LDS.

The other asm in wdt.h (or maybe even more headers or source files) might suffer from the same or similar flaws.

Georg-Johann Lay <gjlayde>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #33933:  wdt-h.diff added by gjlayde (8KiB - text/x-patch - Attached a revised patch that:)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by pitchumani (Posted a comment)
  • -email is unavailable- added by gjlayde (Submitted the item)
  •  

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-06-18 pitchumani StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2015-05-06 gjlayde Attached File- Added wdt-h.diff, #33933
    2014-12-24 pitchumani StatusNone In Progress
        Assigned toNone pitchumani

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code