On Tue, Mar 19, 2024 at 12:38:34PM -0400, David Malcolm wrote:
> On Tue, 2024-03-19 at 16:10 +0100, Stefan Schulze Frielinghaus wrote:
> > On s390 pr94688.c is failing due to excess error
> > 
> > pr94688.c:6:5: warning: allocated buffer size is not a multiple of
> > the pointee's size [CWE-131] [-Wanalyzer-allocation-size]
> > 
> > This is because on s390 functions are by default aligned to an 8-byte
> > boundary and during function type construction size is set to
> > function
> > boundary.  Thus, for the assignment
> > 
> > a.0_1 = (void (*<T237>) ()) &a;
> > 
> > we have that the right-hand side is pointing to a 4-byte memory
> > region
> > whereas the size of the function pointer is 8 byte and a warning is
> > emitted.
> 
> FWIW the test case in question is a regression test for an ICE seen in
> the GCC 10 implementation of the analyzer, which was fixed by the big
> rewrite in r11-2694-g808f4dfeb3a95f.
> 
> So the code in the test doesn't make a great deal of sense.
> 
> > 
> > I could follow and skip this test as done in PR112705, or we could
> > bail
> > out early in the analyzer for function pointers.  My intuition so far
> > is that -Wanalyzer-allocation-size shouldn't care about function
> > pointer.  Therefore, I went for bailing out early.  If you believe
> > this
> > is wrong I can still go by skipping this test on s390.  Any thoughts?
> 
> I tried imagining a situation where we're analyzing a function
> generated at run-time, but it strikes me that the buffer allocated for
> such a function can be of arbitrary size.  So -Wanalyzer-allocation-
> size is meaningless for functions.
> 
> There's probably a case for checking for mismatches between pointers to
> code vs pointers to data (e.g. alignments, Harvard architecture
> machines, etc), but -Wanalyzer-allocation-size doesn't do that.
> 
> So I think your patch is correct.
> 
> OK to push it if it passes bootstrap&regression testing.

Bootstrapped and regtested on x64 and s390x.

Thanks,
Stefan

> 
> Thanks
> Dave
> 
> > ---
> >  gcc/analyzer/region-model.cc | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-
> > model.cc
> > index f079d1fb37e..1b43443d168 100644
> > --- a/gcc/analyzer/region-model.cc
> > +++ b/gcc/analyzer/region-model.cc
> > @@ -3514,6 +3514,10 @@ region_model::check_region_size (const region
> > *lhs_reg, const svalue *rhs_sval,
> >        || TYPE_SIZE_UNIT (pointee_type) == NULL_TREE)
> >      return;
> >  
> > +  /* Bail out early on function pointers.  */
> > +  if (TREE_CODE (pointee_type) == FUNCTION_TYPE)
> > +    return;
> > +
> >    /* Bail out early on pointers to structs where we can
> >       not deduce whether the buffer size is compatible.  */
> >    bool is_struct = RECORD_OR_UNION_TYPE_P (pointee_type);
> 

Reply via email to