shafik added inline comments.
================
Comment at: clang/lib/AST/ExprConstant.cpp:13540
ED->getValueRange(Max, Min);
+ --Max;
----------------
erichkeane wrote:
> shafik wrote:
> > erichkeane wrote:
> > > I don't think this is the correct answer. Even though the other use of
> > > this seems to 'work', `getValueRange` is still returning wrapped values
> > > here. The fix is to figure out how to fix the math in getValueRange, and
> > > change the sanitizer's IR generation if necessary.
> > So the other user of these values is `getRangeForLoadFromType(...)` through
> > `getRangeForType(...)` which is used to generate the [range
> > metadata](https://llvm.org/docs/LangRef.html#range-metadata) and in this
> > case the range is exclusive on the right side:
> >
> > > The pair a,b represents the range [a,b).
> >
> > and it is allowed to wrap:
> >
> > > The range is allowed to wrap.
> >
> > So I believe the behavior will be correct.
> >
> > Even though the range looks off in this example:
> > https://godbolt.org/z/z7d9PKoMn
> >
> > ```
> > !{i32 0, i32 -2147483648}
> > ```
> >
> > I think it does the right thing.
> >
> > Note the language ref also says:
> >
> > > The type must match the type loaded by the instruction.
> >
> > and a quick experiment to use `i33` confirms this breaks.
> Hmm... ok. Well, its quite unfortunate that 2 of our 3 uses of this are
> having to hack around this behavior a bit, but I guess this will have to be
> acceptable short-term then.
I agree, maybe `getValueRange(...)` needs a flag for inclusive Vs exclusive.
That would remove the hack and make it explicit that we have a different
requirement at the call site.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130811/new/
https://reviews.llvm.org/D130811
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
