On 18 May 2017 at 13:28, Dehao Chen <de...@google.com> wrote: > My understanding is that r302938 makes clang generate incorrect code > (clang itself), which lead to unexpected clang behavior. Is it correct? >
Yes, I believe so, specifically when the stage2 clang is built with -fsanitize=memory and (I think) -O2. If yes, how can I reproduce this issue so that I can try to triage/fix the > problem? > I'll provide you with instructions offline. Thanks, > Dehao > > On Thu, May 18, 2017 at 1:22 PM, Richard Smith <rich...@metafoo.co.uk> > wrote: > >> On 18 May 2017 1:19 pm, "Richard Smith" <rich...@metafoo.co.uk> wrote: >> >> On 18 May 2017 1:14 pm, "Dehao Chen" <de...@google.com> wrote: >> >> What's the issue? Build breaking? Performance regression? It's not clear >> from the limit info in this thread... >> >> >> r302938 introduced or exposed a miscompile that causes a stage2 msan >> build of Clang to misbehave: >> >> >> To be fair, we don't know for sure that it's a miscompile. This code >> works with older clang and with other compilers and is clean under the >> sanitizers, but it might still be that there's some UB here. >> >> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-boo >> tstrap/builds/1349/steps/check-clang%20msan/logs/stdio >> >> From my post to another branch of this thread: >> >> I grabbed a clang binary from the build bot and have been trying to >> figure out what's gone wrong. So far it looks like the msan-enabled stage1 >> miscompiled some part of clang's lib/Lex/TokenLexer.cpp (but I'm not sure >> of that). It *looks* like TokenLexer::ExpandFunctionArguments is >> corrupting the Flags member of the token, somewhere around the "if >> (!VaArgsPseudoPaste)" block. I see what looks like a Token::Flags value of >> 0x2150, which is garbage: the highest assigned flag is 0x400. >> >> Dehao >> >> On Thu, May 18, 2017 at 1:02 PM, Vitaly Buka <vitalyb...@google.com> >> wrote: >> >>> Local build: r302937 no issue, r302938 has issue. >>> >>> On Thu, May 18, 2017 at 7:23 AM Dehao Chen <de...@google.com> wrote: >>> >>>> Could you give some context on how r302938 is related to this? >>>> >>>> Thanks, >>>> Dehao >>>> >>>> On Wed, May 17, 2017 at 11:14 PM, Vitaly Buka <vitalyb...@google.com> >>>> wrote: >>>> >>>>> +Dehao Chen <de...@google.com> >>>>> it started from r302938 >>>>> >>>>> On Wed, May 17, 2017 at 8:09 PM Jordan Rose via cfe-commits < >>>>> cfe-commits@lists.llvm.org> wrote: >>>>> >>>>>> Thanks, this is helpful! >>>>>> >>>>>> >>>>>> On May 16, 2017, at 12:26, Richard Smith <rich...@metafoo.co.uk> >>>>>> wrote: >>>>>> >>>>>> On 15 May 2017 at 10:28, Jordan Rose via cfe-commits < >>>>>> cfe-commits@lists.llvm.org> wrote: >>>>>> >>>>>>> Hi, Richard. Swift was using this information in order to put >>>>>>> imported macros in a particular context. It wouldn't surprise me to hear >>>>>>> that we were doing it wrong, and that there's a better way to go from a >>>>>>> macro back to a module, but is there a recommended replacement? >>>>>>> >>>>>> >>>>>> The recommended way to connect macros to modules is via the >>>>>> ModuleMacro objects, which represent a macro exported from a module. You >>>>>> can query the exported macro for a (module, identifier) pair with >>>>>> Preprocessor::getModuleMacro, or walk the ModuleMacro graph for an >>>>>> identifier by starting from Preprocessor::getLeafModuleMacros. >>>>>> >>>>>> If you alternatively want to know the set of macros that would be >>>>>> visible with a given set of imports, after setting up that state you can >>>>>> walk the range produced by Preprocessor::macros(true) and query >>>>>> getActiveModuleMacros on each MacroState. >>>>>> >>>>>> If you want to know "what is the set of macros exported directly by >>>>>> this module?", we don't have a prebuilt mechanism for that, since no >>>>>> in-tree client wants that information, but one way would be to walk >>>>>> macros(true) and query getModuleMacro(module, identifier) on each one. >>>>>> >>>>>> Thanks, >>>>>>> Jordan >>>>>>> >>>>>>> >>>>>>> > On May 12, 2017, at 16:40, Richard Smith via cfe-commits < >>>>>>> cfe-commits@lists.llvm.org> wrote: >>>>>>> > >>>>>>> > Author: rsmith >>>>>>> > Date: Fri May 12 18:40:52 2017 >>>>>>> > New Revision: 302966 >>>>>>> > >>>>>>> > URL: http://llvm.org/viewvc/llvm-project?rev=302966&view=rev >>>>>>> > Log: >>>>>>> > Remove unused tracking of owning module for MacroInfo objects. >>>>>>> > >>>>>>> > Modified: >>>>>>> > cfe/trunk/include/clang/Lex/MacroInfo.h >>>>>>> > cfe/trunk/include/clang/Lex/Preprocessor.h >>>>>>> > cfe/trunk/lib/Lex/MacroInfo.cpp >>>>>>> > cfe/trunk/lib/Lex/PPDirectives.cpp >>>>>>> > cfe/trunk/lib/Lex/Preprocessor.cpp >>>>>>> > cfe/trunk/lib/Serialization/ASTReader.cpp >>>>>>> > cfe/trunk/lib/Serialization/ASTWriter.cpp >>>>>>> > >>>>>>> > Modified: cfe/trunk/include/clang/Lex/MacroInfo.h >>>>>>> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >>>>>>> Lex/MacroInfo.h?rev=302966&r1=302965&r2=302966&view=diff >>>>>>> > ============================================================ >>>>>>> ================== >>>>>>> > --- cfe/trunk/include/clang/Lex/MacroInfo.h (original) >>>>>>> > +++ cfe/trunk/include/clang/Lex/MacroInfo.h Fri May 12 18:40:52 >>>>>>> 2017 >>>>>>> > @@ -105,9 +105,6 @@ class MacroInfo { >>>>>>> > /// \brief Must warn if the macro is unused at the end of >>>>>>> translation unit. >>>>>>> > bool IsWarnIfUnused : 1; >>>>>>> > >>>>>>> > - /// \brief Whether this macro info was loaded from an AST file. >>>>>>> > - bool FromASTFile : 1; >>>>>>> > - >>>>>>> > /// \brief Whether this macro was used as header guard. >>>>>>> > bool UsedForHeaderGuard : 1; >>>>>>> > >>>>>>> > @@ -264,34 +261,16 @@ public: >>>>>>> > IsDisabled = true; >>>>>>> > } >>>>>>> > >>>>>>> > - /// \brief Determine whether this macro info came from an AST >>>>>>> file (such as >>>>>>> > - /// a precompiled header or module) rather than having been >>>>>>> parsed. >>>>>>> > - bool isFromASTFile() const { return FromASTFile; } >>>>>>> > - >>>>>>> > /// \brief Determine whether this macro was used for a header >>>>>>> guard. >>>>>>> > bool isUsedForHeaderGuard() const { return UsedForHeaderGuard; } >>>>>>> > >>>>>>> > void setUsedForHeaderGuard(bool Val) { UsedForHeaderGuard = Val; >>>>>>> } >>>>>>> > >>>>>>> > - /// \brief Retrieve the global ID of the module that owns this >>>>>>> particular >>>>>>> > - /// macro info. >>>>>>> > - unsigned getOwningModuleID() const { >>>>>>> > - if (isFromASTFile()) >>>>>>> > - return *(const unsigned *)(this + 1); >>>>>>> > - >>>>>>> > - return 0; >>>>>>> > - } >>>>>>> > - >>>>>>> > void dump() const; >>>>>>> > >>>>>>> > private: >>>>>>> > unsigned getDefinitionLengthSlow(const SourceManager &SM) const; >>>>>>> > >>>>>>> > - void setOwningModuleID(unsigned ID) { >>>>>>> > - assert(isFromASTFile()); >>>>>>> > - *(unsigned *)(this + 1) = ID; >>>>>>> > - } >>>>>>> > - >>>>>>> > friend class Preprocessor; >>>>>>> > }; >>>>>>> > >>>>>>> > >>>>>>> > Modified: cfe/trunk/include/clang/Lex/Preprocessor.h >>>>>>> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >>>>>>> Lex/Preprocessor.h?rev=302966&r1=302965&r2=302966&view=diff >>>>>>> > ============================================================ >>>>>>> ================== >>>>>>> > --- cfe/trunk/include/clang/Lex/Preprocessor.h (original) >>>>>>> > +++ cfe/trunk/include/clang/Lex/Preprocessor.h Fri May 12 >>>>>>> 18:40:52 2017 >>>>>>> > @@ -644,14 +644,6 @@ class Preprocessor { >>>>>>> > /// of that list. >>>>>>> > MacroInfoChain *MIChainHead; >>>>>>> > >>>>>>> > - struct DeserializedMacroInfoChain { >>>>>>> > - MacroInfo MI; >>>>>>> > - unsigned OwningModuleID; // MUST be immediately after the >>>>>>> MacroInfo object >>>>>>> > - // so it can be accessed by >>>>>>> MacroInfo::getOwningModuleID(). >>>>>>> > - DeserializedMacroInfoChain *Next; >>>>>>> > - }; >>>>>>> > - DeserializedMacroInfoChain *DeserialMIChainHead; >>>>>>> > - >>>>>>> > void updateOutOfDateIdentifier(IdentifierInfo &II) const; >>>>>>> > >>>>>>> > public: >>>>>>> > @@ -1669,10 +1661,6 @@ public: >>>>>>> > /// \brief Allocate a new MacroInfo object with the provided >>>>>>> SourceLocation. >>>>>>> > MacroInfo *AllocateMacroInfo(SourceLocation L); >>>>>>> > >>>>>>> > - /// \brief Allocate a new MacroInfo object loaded from an AST >>>>>>> file. >>>>>>> > - MacroInfo *AllocateDeserializedMacroInfo(SourceLocation L, >>>>>>> > - unsigned SubModuleID); >>>>>>> > - >>>>>>> > /// \brief Turn the specified lexer token into a fully checked >>>>>>> and spelled >>>>>>> > /// filename, e.g. as an operand of \#include. >>>>>>> > /// >>>>>>> > @@ -1764,9 +1752,6 @@ private: >>>>>>> > /// macro name. >>>>>>> > void updateModuleMacroInfo(const IdentifierInfo *II, >>>>>>> ModuleMacroInfo &Info); >>>>>>> > >>>>>>> > - /// \brief Allocate a new MacroInfo object. >>>>>>> > - MacroInfo *AllocateMacroInfo(); >>>>>>> > - >>>>>>> > DefMacroDirective *AllocateDefMacroDirective(MacroInfo *MI, >>>>>>> > SourceLocation Loc); >>>>>>> > UndefMacroDirective *AllocateUndefMacroDirective(SourceLocation >>>>>>> UndefLoc); >>>>>>> > >>>>>>> > Modified: cfe/trunk/lib/Lex/MacroInfo.cpp >>>>>>> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/MacroI >>>>>>> nfo.cpp?rev=302966&r1=302965&r2=302966&view=diff >>>>>>> > ============================================================ >>>>>>> ================== >>>>>>> > --- cfe/trunk/lib/Lex/MacroInfo.cpp (original) >>>>>>> > +++ cfe/trunk/lib/Lex/MacroInfo.cpp Fri May 12 18:40:52 2017 >>>>>>> > @@ -29,7 +29,6 @@ MacroInfo::MacroInfo(SourceLocation DefL >>>>>>> > IsUsed(false), >>>>>>> > IsAllowRedefinitionsWithoutWarning(false), >>>>>>> > IsWarnIfUnused(false), >>>>>>> > - FromASTFile(false), >>>>>>> > UsedForHeaderGuard(false) { >>>>>>> > } >>>>>>> > >>>>>>> > @@ -137,7 +136,6 @@ LLVM_DUMP_METHOD void MacroInfo::dump() >>>>>>> > if (IsAllowRedefinitionsWithoutWarning) >>>>>>> > Out << " allow_redefinitions_without_warning"; >>>>>>> > if (IsWarnIfUnused) Out << " warn_if_unused"; >>>>>>> > - if (FromASTFile) Out << " imported"; >>>>>>> > if (UsedForHeaderGuard) Out << " header_guard"; >>>>>>> > >>>>>>> > Out << "\n #define <macro>"; >>>>>>> > >>>>>>> > Modified: cfe/trunk/lib/Lex/PPDirectives.cpp >>>>>>> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDire >>>>>>> ctives.cpp?rev=302966&r1=302965&r2=302966&view=diff >>>>>>> > ============================================================ >>>>>>> ================== >>>>>>> > --- cfe/trunk/lib/Lex/PPDirectives.cpp (original) >>>>>>> > +++ cfe/trunk/lib/Lex/PPDirectives.cpp Fri May 12 18:40:52 2017 >>>>>>> > @@ -54,35 +54,12 @@ using namespace clang; >>>>>>> > // Utility Methods for Preprocessor Directive Handling. >>>>>>> > //===------------------------------------------------------- >>>>>>> ---------------===// >>>>>>> > >>>>>>> > -MacroInfo *Preprocessor::AllocateMacroInfo() { >>>>>>> > - MacroInfoChain *MIChain = BP.Allocate<MacroInfoChain>(); >>>>>>> > - MIChain->Next = MIChainHead; >>>>>>> > +MacroInfo *Preprocessor::AllocateMacroInfo(SourceLocation L) { >>>>>>> > + auto *MIChain = new (BP) MacroInfoChain{L, MIChainHead}; >>>>>>> > MIChainHead = MIChain; >>>>>>> > return &MIChain->MI; >>>>>>> > } >>>>>>> > >>>>>>> > -MacroInfo *Preprocessor::AllocateMacroInfo(SourceLocation L) { >>>>>>> > - MacroInfo *MI = AllocateMacroInfo(); >>>>>>> > - new (MI) MacroInfo(L); >>>>>>> > - return MI; >>>>>>> > -} >>>>>>> > - >>>>>>> > -MacroInfo *Preprocessor::AllocateDeserializedMacroInfo(SourceLocation >>>>>>> L, >>>>>>> > - unsigned >>>>>>> SubModuleID) { >>>>>>> > - static_assert(alignof(MacroInfo) >= sizeof(SubModuleID), >>>>>>> > - "alignment for MacroInfo is less than the ID"); >>>>>>> > - DeserializedMacroInfoChain *MIChain = >>>>>>> > - BP.Allocate<DeserializedMacroInfoChain>(); >>>>>>> > - MIChain->Next = DeserialMIChainHead; >>>>>>> > - DeserialMIChainHead = MIChain; >>>>>>> > - >>>>>>> > - MacroInfo *MI = &MIChain->MI; >>>>>>> > - new (MI) MacroInfo(L); >>>>>>> > - MI->FromASTFile = true; >>>>>>> > - MI->setOwningModuleID(SubModuleID); >>>>>>> > - return MI; >>>>>>> > -} >>>>>>> > - >>>>>>> > DefMacroDirective *Preprocessor::AllocateDefMacroDirective(MacroInfo >>>>>>> *MI, >>>>>>> > >>>>>>> SourceLocation Loc) { >>>>>>> > return new (BP) DefMacroDirective(MI, Loc); >>>>>>> > >>>>>>> > Modified: cfe/trunk/lib/Lex/Preprocessor.cpp >>>>>>> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Prepro >>>>>>> cessor.cpp?rev=302966&r1=302965&r2=302966&view=diff >>>>>>> > ============================================================ >>>>>>> ================== >>>>>>> > --- cfe/trunk/lib/Lex/Preprocessor.cpp (original) >>>>>>> > +++ cfe/trunk/lib/Lex/Preprocessor.cpp Fri May 12 18:40:52 2017 >>>>>>> > @@ -88,7 +88,7 @@ Preprocessor::Preprocessor(std::shared_p >>>>>>> > CurDirLookup(nullptr), CurLexerKind(CLK_Lexer), >>>>>>> > CurLexerSubmodule(nullptr), Callbacks(nullptr), >>>>>>> > CurSubmoduleState(&NullSubmoduleState), >>>>>>> MacroArgCache(nullptr), >>>>>>> > - Record(nullptr), MIChainHead(nullptr), >>>>>>> DeserialMIChainHead(nullptr) { >>>>>>> > + Record(nullptr), MIChainHead(nullptr) { >>>>>>> > OwnsHeaderSearch = OwnsHeaders; >>>>>>> > >>>>>>> > CounterValue = 0; // __COUNTER__ starts at 0. >>>>>>> > @@ -169,11 +169,6 @@ Preprocessor::~Preprocessor() { >>>>>>> > std::fill(TokenLexerCache, TokenLexerCache + >>>>>>> NumCachedTokenLexers, nullptr); >>>>>>> > CurTokenLexer.reset(); >>>>>>> > >>>>>>> > - while (DeserializedMacroInfoChain *I = DeserialMIChainHead) { >>>>>>> > - DeserialMIChainHead = I->Next; >>>>>>> > - I->~DeserializedMacroInfoChain(); >>>>>>> > - } >>>>>>> > - >>>>>>> > // Free any cached MacroArgs. >>>>>>> > for (MacroArgs *ArgList = MacroArgCache; ArgList;) >>>>>>> > ArgList = ArgList->deallocate(); >>>>>>> > >>>>>>> > Modified: cfe/trunk/lib/Serialization/ASTReader.cpp >>>>>>> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serializat >>>>>>> ion/ASTReader.cpp?rev=302966&r1=302965&r2=302966&view=diff >>>>>>> > ============================================================ >>>>>>> ================== >>>>>>> > --- cfe/trunk/lib/Serialization/ASTReader.cpp (original) >>>>>>> > +++ cfe/trunk/lib/Serialization/ASTReader.cpp Fri May 12 18:40:52 >>>>>>> 2017 >>>>>>> > @@ -1534,9 +1534,8 @@ MacroInfo *ASTReader::ReadMacroRecord(Mo >>>>>>> > return Macro; >>>>>>> > >>>>>>> > unsigned NextIndex = 1; // Skip identifier ID. >>>>>>> > - SubmoduleID SubModID = getGlobalSubmoduleID(F, >>>>>>> Record[NextIndex++]); >>>>>>> > SourceLocation Loc = ReadSourceLocation(F, Record, >>>>>>> NextIndex); >>>>>>> > - MacroInfo *MI = PP.AllocateDeserializedMacroInfo(Loc, >>>>>>> SubModID); >>>>>>> > + MacroInfo *MI = PP.AllocateMacroInfo(Loc); >>>>>>> > MI->setDefinitionEndLoc(ReadSourceLocation(F, Record, >>>>>>> NextIndex)); >>>>>>> > MI->setIsUsed(Record[NextIndex++]); >>>>>>> > MI->setUsedForHeaderGuard(Record[NextIndex++]); >>>>>>> > >>>>>>> > Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp >>>>>>> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serializat >>>>>>> ion/ASTWriter.cpp?rev=302966&r1=302965&r2=302966&view=diff >>>>>>> > ============================================================ >>>>>>> ================== >>>>>>> > --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original) >>>>>>> > +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Fri May 12 18:40:52 >>>>>>> 2017 >>>>>>> > @@ -2413,7 +2413,6 @@ void ASTWriter::WritePreprocessor(const >>>>>>> > } >>>>>>> > >>>>>>> > AddIdentifierRef(Name, Record); >>>>>>> > - Record.push_back(inferSubmoduleIDFromLocation(MI->getDefinit >>>>>>> ionLoc())); >>>>>>> > AddSourceLocation(MI->getDefinitionLoc(), Record); >>>>>>> > AddSourceLocation(MI->getDefinitionEndLoc(), Record); >>>>>>> > Record.push_back(MI->isUsed()); >>>>>>> > >>>>>>> > >>>>>>> > _______________________________________________ >>>>>>> > cfe-commits mailing list >>>>>>> > cfe-commits@lists.llvm.org >>>>>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>>>>> >>>>>>> _______________________________________________ >>>>>>> cfe-commits mailing list >>>>>>> cfe-commits@lists.llvm.org >>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>>>>> >>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> cfe-commits mailing list >>>>>> cfe-commits@lists.llvm.org >>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>>>> >>>>> >>>> >> >> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
