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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
