Re: [Xen-devel] [PATCH 7/7] tools/kdd: mute spurious gcc warning

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 04/06/2018 09:41 AM, Wei Liu wrote:
> On Fri, Apr 06, 2018 at 09:39:50AM -0400, Boris Ostrovsky wrote:
>> On 04/06/2018 09:07 AM, Wei Liu wrote:
>>> On Fri, Apr 06, 2018 at 08:39:53AM -0400, Boris Ostrovsky wrote:
>>>> On 04/04/2018 09:50 PM, Marek Marczykowski-Górecki wrote:
>>>>> gcc-8 complains:
>>>>>
>>>>>     kdd.c:698:13: error: 'memcpy' offset [-204, -717] is out of the 
>>>>> bounds [0, 216] of object 'ctrl' with type 'kdd_ctrl' {aka 'union 
>>>>> <anonymous>'} [-Werror=array-bounds]
>>>>>                  memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
>>>>>                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>>>>     kdd.c: In function 'kdd_select_callback':
>>>>>     kdd.c:642:14: note: 'ctrl' declared here
>>>>>          kdd_ctrl ctrl;
>>>>>                   ^~~~
>>>>>
>>>>> But this is impossible - 'offset' is unsigned and correctly validated
>>>>> few lines before.
>>>>>
>>>>> Signed-off-by: Marek Marczykowski-Górecki 
>>>>> <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
>>>>> ---
>>>>>  tools/debugger/kdd/kdd.c | 3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
>>>>> index 1bd5dd5..61d769e 100644
>>>>> --- a/tools/debugger/kdd/kdd.c
>>>>> +++ b/tools/debugger/kdd/kdd.c
>>>>> @@ -695,7 +695,10 @@ static void kdd_handle_read_ctrl(kdd_state *s)
>>>>>              KDD_LOG(s, "Request outside of known control space\n");
>>>>>              len = 0;
>>>>>          } else {
>>>>> +#pragma GCC diagnostic push
>>>>> +#pragma GCC diagnostic ignored "-Warray-bounds"
>>>>>              memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
>>>>> +#pragma GCC diagnostic pop
>>>>>          }
>>>>>      }
>>>>>  
>>>> Breaks 32-bit build, at least with my (ancient, gcc version 4.4.5
>>>> 20101112 (Red Hat 4.4.5-2) (GCC)) compiler:
>>>>
>>>>
>>>>
>>>> kdd.c: In function ‘kdd_handle_read_ctrl’:
>>>> kdd.c:698: error: #pragma GCC diagnostic not allowed inside functions
>>>> kdd.c:699: error: #pragma GCC diagnostic not allowed inside functions
>>>> kdd.c:701: error: #pragma GCC diagnostic not allowed inside functions
>>>> make[5]: *** [kdd.o] Error 1
>>>>
>>> Does moving the relevant #pragma's outside of the function fix it?
>> The additional problem with these pragmas is that apparently push/pop
>> have been introduced in gcc 4.6.0:
>>
>> https://gcc.gnu.org/onlinedocs/gcc-4.6.0/gcc/Diagnostic-Pragmas.html#Diagnostic-Pragmas
>>
>> If you change release number to a lower one (e.g. 4.5.4) you won't see them.
>>
>> So I can move "diagnostic ignored" from inside the function and that
>> will clear the "GCC diagnostic not allowed inside functions" error. But
>> then push/pop are not recognized:
>>
>> cc1: warnings being treated as errors
>> kdd.c:639: error: expected [error|warning|ignored] after ‘#pragma GCC
>> diagnostic’
>> kdd.c:714: error: expected [error|warning|ignored] after ‘#pragma GCC
>> diagnostic’
>>
>> (Interestingly, my 64-bit build completed without issues)
> Hmm... this is messy.
>
> If you have information about which version does what we can try to
> enclose the #pragma's with #if __GCC__.


Can we instead pre-compute the pointer to pacify the compiler? I haven't
seen the original error so I can't test it, but something like

diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
index 61d769e..1b048ac 100644
--- a/tools/debugger/kdd/kdd.c
+++ b/tools/debugger/kdd/kdd.c
@@ -688,6 +688,7 @@ static void kdd_handle_read_ctrl(kdd_state *s)
     } else {
         /* 32-bit control-register space starts at 0x[2]cc, for 84 bytes */
         uint64_t offset = addr;
+        void *ptr = &ctrl.c32;
         if (offset > 0x200)
             offset -= 0x200;
         offset -= 0xcc;
@@ -695,10 +696,8 @@ static void kdd_handle_read_ctrl(kdd_state *s)
             KDD_LOG(s, "Request outside of known control space\n");
             len = 0;
         } else {
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Warray-bounds"
-            memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
-#pragma GCC diagnostic pop
+            ptr += offset;
+            memcpy(buf, ptr, len);
         }
     }
 
-boris

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel