[PATCH] D50930: Move Itanium demangler implementation into a header file and add visitation support.
Dávid Bolvanský via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Nov 2 12:02:15 PDT 2019
xbolva00 added inline comments.
================
Comment at: llvm/trunk/include/llvm/Demangle/ItaniumDemangle.h:609
+ assert(FirstChar == SecondChar);
+ ++FirstChar, ++SecondChar;
+ }
----------------
erik.pilkington wrote:
> xbolva00 wrote:
> > Hi Richard, this code looks suspicious. Is this correct?
> >
> > PVS warns:
> > /home/xbolva00/LLVM/llvm-project/llvm/include/llvm/Demangle/ItaniumDemangle.h 620 err V769 The 'SecondChar' pointer in the '++ SecondChar' expression equals nullptr. The resulting value is senseless and it should not be used.
> >
> >
> (This is my fault, Richard just copied this)
>
> This is sorta fine, this done to distinguish between an empty StringView with nullptr first and last and an empty NodeOrString. The pointers are never loaded from, but I believe this is still UB. That being said, I think this class is over engineered and we'd probably be better off putting the StringView in a `NameType` in cases where we need to have either a Node or a String.
Thanks for info. Can you add code comment here for now?
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D50930/new/
https://reviews.llvm.org/D50930
More information about the llvm-commits
mailing list