Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

ADT/SmallVector.h: array subscript 1 is outside array bounds #43696

Open Quuxplusone opened 4 years ago

Quuxplusone commented 4 years ago
Bugzilla Link PR44726
Status NEW
Importance P enhancement
Reported by Gabor Greif (ggreif@gmail.com)
Reported on 2020-01-31 02:08:29 -0800
Last modified on 2020-09-27 03:39:24 -0700
Version 10.0
Hardware PC All
CC dblaikie@gmail.com, efriedma@quicinc.com, kim.grasman@gmail.com, llvm-bugs@lists.llvm.org
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

While compiling the 10.0.0rc1 on our nix builder:

/build/llvm/include/llvm/ADT/SmallVector.h: In member function 'void llvm::ExecutionDomainFix::visitSoftInstr(llvm::MachineInstr*, unsigned int)':
/build/llvm/include/llvm/ADT/SmallVector.h:520:7: warning: array subscript 1 is outside array bounds of 'int [1]' [-Warray-bounds]
  520 |       ++EltPtr;
      |       ^~
Quuxplusone commented 4 years ago
-- The C compiler identification is GNU 9.2.0
-- The CXX compiler identification is GNU 9.2.0

On the linux builder.
Quuxplusone commented 4 years ago

Looks like a false positive warning to me - but you'd have to reduce it further to prove that before filing a bug with GCC if you wanted to.

But incrementing a pointer to a single object isn't UB - you can have a pointer to "one past" an object.

Quuxplusone commented 4 years ago
Actually, the code in SmallVector.h looks suspicious to me.  It's attempting to
allow something like "vec.insert(vec.begin(), *vec.begin())", but I'm pretty
sure it doesn't work right: it isn't correctly taking into account the
possibility of the vector growing.

I'm not sure that's related to the warning, though; if it were truly detecting
that issue, it would probably print a different warning.
Quuxplusone commented 4 years ago
(In reply to Eli Friedman from comment #3)
> Actually, the code in SmallVector.h looks suspicious to me.  It's attempting
> to allow something like "vec.insert(vec.begin(), *vec.begin())", but I'm
> pretty sure it doesn't work right: it isn't correctly taking into account
> the possibility of the vector growing.

Oh, yeah - I think I went down that rabbit hole a few years ago & eventually
gave up thinking about what scenarios it should/could support. I should've
written some notes. But I'm pretty sure it claims to make some things work that
don't work.

> I'm not sure that's related to the warning, though; if it were truly
> detecting that issue, it would probably print a different warning.

Yeah - pretty sure it isn't, but could be wrong.