aleksandr.urakov added inline comments.
Comment at: lit/SymbolFile/PDB/class-layout.test:3
+RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/ClassLayoutTest.cpp /o
%T/ClassLayoutTest.cpp.obj
+RUN: link %T/ClassLayoutTest.cpp.obj /DEBUG /nodefaultlib /ENTRY:main
/OUT:%T/ClassLayoutTest.
Hui added inline comments.
Comment at: lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp:37
+ };
+ union { // Test unnamed union. MSVC treats it as `int a; float b;`
+int a;
aleksandr.urakov wrote:
> Here is a problem. `MicrosoftRecordLayoutBuilder` asserts ev
Hui added inline comments.
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:696
+ base_ast_type.GetOpaqueQualType(), access,
+ pdb_base_class->isVirtualBaseClass(), /*base_of_class*/ true);
+ base_classes.push_back(base_spec);
ale
aleksandr.urakov added inline comments.
Comment at: lit/SymbolFile/PDB/Inputs/ClassLayoutTest.cpp:37
+ };
+ union { // Test unnamed union. MSVC treats it as `int a; float b;`
+int a;
Hui wrote:
> aleksandr.urakov wrote:
> > Here is a problem. `MicrosoftRec
I think the problem is in lld, we don’t emit S_UDT symbols because it
caused weird problems in WinDbg. There’s a comment explaining it in
PDB.cpp. But after some recent fixes this may just work. So we should
probably try again to emit the S_UDT in lld, I think that should fix it
On Fri, Jul 20,
zturner added a comment.
I think the problem is in lld, we don’t emit S_UDT symbols because it
caused weird problems in WinDbg. There’s a comment explaining it in
PDB.cpp. But after some recent fixes this may just work. So we should
probably try again to emit the S_UDT in lld, I think that shou
labath added a subscriber: lemo.
labath added a comment.
@markmentovai, @lemo, do you know under which circumstances do these extra 4
bytes get emitted? Is there any chance we could document this better than just
saying "sometimes"?
Comment at: unittests/Process/minidump/Mini
labath added a comment.
In https://reviews.llvm.org/D48351#1168384, @jingham wrote:
> If this pattern becomes more common, then we have to deal with how somebody
> would find these dump routines. If RegisterValue gets moved out of Core,
> would you really look to a file in Core for the dump ro
I had previously thought of making a top level project called Dump that
depends on everything. Also makes it very obvious where all the dumpers
are. It can have overloaded functions called lldb_private::dump(T&) for
every value of T, then no matter what type you have, all you have to do is
call dum
zturner added a subscriber: labath.
zturner added a comment.
I had previously thought of making a top level project called Dump that
depends on everything. Also makes it very obvious where all the dumpers
are. It can have overloaded functions called lldb_private::dump(T&) for
every value of T, the
labath added subscribers: clayborg, jingham, zturner.
labath added a comment.
Well, if it depends on everything, then it can only used from places
that also depend on everything. Right now I don't think it matters now
(though it will create a new Dump <-> "everything calling Dump"
cycle). However,
We only use dumping from lldb-test, from inside the debugger, or from the
top level command interpreter. All of those things necessarily depend on
everything anyway.
On Fri, Jul 20, 2018 at 8:02 AM Pavel Labath wrote:
> Well, if it depends on everything, then it can only used from places
> that a
Well, if it depends on everything, then it can only used from places
that also depend on everything. Right now I don't think it matters now
(though it will create a new Dump <-> "everything calling Dump"
cycle). However, I can imagine this might cause some tricky situations
later on when we try to
This is the current list of files (after this patch) that call
DumpDataExtractor or DumpRegisterValue
source/API/SBData.cpp
source/Commands/CommandObjectMemory.cpp
source/Commands/CommandObjectRegister.cpp
source/Core/Address.cpp
source/Core/EmulateInstruction.cpp
source/Core/Event.cpp
source/Core
jingham added a comment.
Dump is really meant to be the private description of the object that you would
use for logging and the like - Description was the public face of a class. So
while the Description-like functionality could have a restrictions that
constrain its use to pretty high up in
clayborg added a comment.
The "sometimes" depends on how compilers pad the structures that define the
lists. Since the structs look like:
struct MemoryList {
uint32_t count;
MemoryDescriptor descriptors[];
};
The compiler might end up padding and extra 4 bytes depending on what is i
markmentovai added a comment.
This has got to be padding.
Breakpad’s structure definitions don’t nail down alignment as they probably
should. A 32-bit system writing an MDRawMemoryList will write what’s intended,
but a 64-bit one will need to insert four bytes of padding after
number_of_memory
clayborg added a comment.
In https://reviews.llvm.org/D49579#1170099, @markmentovai wrote:
> This has got to be padding.
>
> Breakpad’s structure definitions don’t nail down alignment as they probably
> should. A 32-bit system writing an MDRawMemoryList will write what’s
> intended, but a 64-bi
clayborg updated this revision to Diff 156544.
clayborg added a comment.
Herald added subscribers: mgorny, srhines.
Improve comments to indicate why padding might be there, fix a test case to use
the right file, and add all new .dmp files to the CMakeLists.txt
https://reviews.llvm.org/D49579
F
clayborg updated this revision to Diff 156545.
clayborg added a comment.
Remove xcode project changes.
https://reviews.llvm.org/D49579
Files:
source/Plugins/Process/minidump/MinidumpTypes.cpp
unittests/Process/minidump/CMakeLists.txt
unittests/Process/minidump/Inputs/memory-list-not-padde
clayborg added a comment.
BTW: The padding fix was inspired from the breakpad sources as they do this
exact same thing.
https://reviews.llvm.org/D49579
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/
markmentovai added a comment.
I know, that was a mistake.
(Unfortunately, I reviewed it. 11 years ago. And now I feel responsible for all
of those malformed minidumps floating around out there.)
If you do need to do this, it seems fine to me, since it’s basically the same
exact workaround. If
clayborg added a comment.
Thanks for the info. Pavel? This good to go? I have another patch that adds the
ARM and ARM64 support that will quickly follow.
https://reviews.llvm.org/D49579
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http
friss added a comment.
Stefan, you should always add lldb-commits to the subscribers of your phab
reviews. I added it now, but IIRC there are issues with adding subscribers
after the fact.
https://reviews.llvm.org/D49612
___
lldb-commits mailing l
sgraenitz added inline comments.
Comment at: source/Core/Mangled.cpp:310
+#elif defined(LLDB_USE_LLVM_DEMANGLER)
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);
sgraenitz wrote:
> erik.pilkington wrote:
davide added a comment.
Thanks for doing this.
We may consider doing some A-B testing between the two demanglers.
A strategy that worked very well for similar purposes was that of running `nm`
on a large executable (e.g. clang or lldb itself) and see whether we demangle
in the same exact way and
26 matches
Mail list logo
