[Lldb-commits] [PATCH] D83881: [lldb/COFF] Remove strtab zeroing hack

Fri Jul 17 04:03:27 PDT 2020

labath marked 2 inline comments as done.
labath added a comment.

Thanks for checking this out. Sorry for forgetting you Martin, I'll have to remember to add you to my future coff patches.

================
Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:672
                 break;
               symbol_name.assign(symbol_name_cstr, sizeof(symbol.name));
             }
----------------
mstorsjo wrote:
> Kind of unrelated question: Does this treat the assigned symbol as always 8 bytes long, or does it scan the provided 8 bytes for potential trailing terminators? Object/COFFObjectFile has got a bit of extra logic to distinguish between those cases: https://github.com/llvm/llvm-project/blob/master/llvm/lib/Object/COFFObjectFile.cpp#L999-L1004
It always treats is as 8 bytes, but then the string is accessed via `.c_str()` on line 679. which picks up the first nul character in the string, which is either one of the embedded ones, or the one that std::string appends internally.

I don't know if that was intentional -- it's kinda neat, but also quite scary.

================
Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:689
               i += symbol.naux;
               offset += symbol_size;
             }
----------------
mstorsjo wrote:
> Unrelated to the patch at hand: I believe this should be `offset += symbol.naux * symbol_size;`. I could try to make a patch to exercise that case at some later time...
Yeah, that looks like a bug. If it is, we could still manage to cherry-pick that for 11.0.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83881/new/

https://reviews.llvm.org/D83881