[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] RFR: 8266171: -Warray-bounds happens in imageioJPEG.c
From:       Phil Race <prr () openjdk ! java ! net>
Date:       2021-04-30 18:20:50
Message-ID: rjX2DTlE5piyOY-6IL4n2QYC-w8ar6iKSCYUe5Vc6wk=.9b46db83-d283-4196-b998-266925193089 () github ! com
[Download RAW message or body]

On Fri, 30 Apr 2021 00:18:15 GMT, Yasumasa Suenaga <ysuenaga@openjdk.org> wrote:

> > src/java.desktop/share/native/libjavajpeg/imageioJPEG.c line 673:
> > 
> > > 671:         if (info->is_decompressor) {
> > > 672:             j_decompress_ptr dinfo = (j_decompress_ptr) info;
> > > 673: #ifdef __GNUC__
> > 
> > I know how these structs are defined but I am not sure how gcc can decide \
> > anything like this. I'd almost worry if it were true that we had the other type \
> > despite what the flag said except I can't imagine gcc is doing even any static \
> > analysis of the code calling sequence and you may even need a dynamic analysis \
> > for this. 
> > Have you submitted a gcc bug ?
> > Why is it only complaining in this branch ?
> > Have you considered disabling the warning in the make files - with broader scope \
> > of course - but a simpler change ? 
> > Is 520 bytes the actual size of the compress struct ? And even then I am not sure \
> > I know what the compiler message means. so long as we have the right starting \
> > address free will only free what was allocated ... surely ...
> 
> I also think GCC does not do any static analysis of the code calling. And also the \
> code uses `struct` like a C++ class, so I guess GCC reports incorrect warnings at \
> this point. Many -Warray-bounds related issues are reported in [GCC \
> Bugzilla](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56456). Especially it does \
> not seem to work when the value is accessed by a pointer \
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99474 - it looks alike to me. 
> WebKit seems to encounter [similar \
> issue](https://bugs.webkit.org/show_bug.cgi?id=224782), they has avoided it with \
> pragma. 
> I concidered to disable it with a compiler option in makefile of course as you \
> said, but I didn't do so because I heard in other review (I forget the JBS ticket) \
> that we should disable warnings locally because they might be useful in future.

In this case the rest of the code here is unlikely to change so I think it is fine to \
put it in the build and it will be easier to revert it when gcc is fixed .. and since \
the warning looks like it is bogus more often than not wider blocking of it is \
probably OK anyway. But I am also sure this is the only case.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3788


[prev in list] [next in list] [prev in thread] [next in thread]