mstorsjo added inline comments.
================ Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:828 + // either names will work. Only synchronize the symbol type. + if (symbol.GetType() == lldb::eSymbolTypeInvalid) + symbol.SetType(exported->GetType()); ---------------- alvinhochun wrote: > mstorsjo wrote: > > alvinhochun wrote: > > > mstorsjo wrote: > > > > This condition had me puzzled for a moment, until I realized that > > > > you're synchronizing symbol type in the other direction here. Is it > > > > worth clarifying this in the comment? > > > > > > > > (Also, I presume the test case doesn't really trigger this case?) > > > Hmm, I can try to improve the comment. > > > > > > The `aliasInt` symbol should trigger this case. Perhaps it will be > > > clearer if I update the previous patch to include these symbols, so the > > > difference in output can be seen in this patch. > > Oh, ok, I had missed that `MapSymbolType` returns code or invalid - I had > > expected it to return code or data. I guess this is fine then. (Changing > > that function, or changing the logic here to look at section types is out > > of scope here anyway, and I don’t know if the difference between data and > > invalid matters?) > Data vs Invalid does seem to have an effect on whether a symbol can be > printed as data or have its address taken. I can't print or dump any > static/globals when the type is Invalid. I think the DWARF debug info should > have at least some information about these symbols, but I have no idea how > that works. > > I wonder if I should change `MapSymbolType` to return Data for anything that > is not code? I think that might be a good idea - but that's clearly a separate change from this one too. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134426/new/ https://reviews.llvm.org/D134426 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
