check to see if null pointer is dereferenceable [PR102630]

  When determining the size of an object compute_objsize_r() assumes
that addresses derived from null pointers are not derefernceable
because null pointers themselves are not, without calling
targetm.addr_space.zero_address_valid() to see if that assumption
is supported for the pointer on the target.  The attached change
adds the missing call to avoid a spurious warning in Glibc that
uses named address spaces to implement TLS on x86_64.

Tested on x86_64-linux.

Martin
  

The testcase uses the __seg_fs address space, which is x86-specific, but 
it isn't in an x86-specific directory or otherwise restricted to x86 
targets; thus, I'd expect it to fail for other architectures.

This is not a review of the rest of the patch.
  

On 10/11/21 6:26 PM, Joseph Myers wrote:
> The testcase uses the __seg_fs address space, which is x86-specific, but
> it isn't in an x86-specific directory or otherwise restricted to x86
> targets; thus, I'd expect it to fail for other architectures.
> 
> This is not a review of the rest of the patch.
> 

Good point!  I thought I might make the test target-independent
(via macros) but it looks like just i386 defines the hook to
something other than false so I should probably move it under
i386.

Thanks
Martin
  

On Wed, Oct 13, 2021 at 3:32 AM Martin Sebor via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On 10/11/21 6:26 PM, Joseph Myers wrote:
> > The testcase uses the __seg_fs address space, which is x86-specific, but
> > it isn't in an x86-specific directory or otherwise restricted to x86
> > targets; thus, I'd expect it to fail for other architectures.
> >
> > This is not a review of the rest of the patch.
> >
>
> Good point!  I thought I might make the test target-independent
> (via macros) but it looks like just i386 defines the hook to
> something other than false so I should probably move it under
> i386.

The patch is OK with the testcase moved.

Note I don't think we should warn about *(int *)0xdeadbee0,

       /* Pointer constants other than null are most likely the result
-        of erroneous null pointer addition/subtraction.  Set size to
-        zero.  For null pointers, set size to the maximum for now
-        since those may be the result of jump threading.  */

there's too much "may be" and "most likely" for my taste.  How can
the user mark a deliberate valid constant address?

Maybe we can use better (target dependent?) heuristic based on
what virtual addresses are likely unmapped (the zero page, the
page "before" the zero page)?

Richard.


> Thanks
> Martin
  

On 10/13/21 2:25 AM, Richard Biener wrote:
> On Wed, Oct 13, 2021 at 3:32 AM Martin Sebor via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> On 10/11/21 6:26 PM, Joseph Myers wrote:
>>> The testcase uses the __seg_fs address space, which is x86-specific, but
>>> it isn't in an x86-specific directory or otherwise restricted to x86
>>> targets; thus, I'd expect it to fail for other architectures.
>>>
>>> This is not a review of the rest of the patch.
>>>
>>
>> Good point!  I thought I might make the test target-independent
>> (via macros) but it looks like just i386 defines the hook to
>> something other than false so I should probably move it under
>> i386.
> 
> The patch is OK with the testcase moved.

I changed the test, moved it under the i386 directory, and also
added -Wall to the existing addr-space-2.c, and committed
the result in r12-4376.

> 
> Note I don't think we should warn about *(int *)0xdeadbee0,
> 
>         /* Pointer constants other than null are most likely the result
> -        of erroneous null pointer addition/subtraction.  Set size to
> -        zero.  For null pointers, set size to the maximum for now
> -        since those may be the result of jump threading.  */
> 
> there's too much "may be" and "most likely" for my taste.  How can
> the user mark a deliberate valid constant address?

Using a volatile pointer works

   int* volatile p = (int*)0xdeadbee0;
   *p = 0;

but is not very elegant.  The other common solution is to make
it a variable and assign it an address in a linker script, but
that's too heavy-weight for some.

I would prefer to make AVR's attribute address generic and
encourage programmers to switch to using it to declare these
things as objects.  The major advantage is that what's at such
an address becomes a first class citizen in the type system
(and beyond), with a type and size.

> 
> Maybe we can use better (target dependent?) heuristic based on
> what virtual addresses are likely unmapped (the zero page, the
> page "before" the zero page)?

I agree we need something better: ideally, detect the null
pointer arithmetic before it's folded into a constant pointer
address.  I've opened pr102731 as a reminder.

Martin

> 
> Richard.
> 
> 
>> Thanks
>> Martin