Page MenuHomePhabricator

paolosev (Paolo Severini)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 12 2019, 11:43 AM (40 w, 2 d)

Recent Activity

Jun 18 2020

paolosev added a comment to D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.

What about replacing ProcessReadMemory(addr_t addr, ...) with ProcessReadMemory(Address addr, ...), or even banning the use of lldb::addr_t on everywhere except in the bowels of Process subclasses and as an interchange for getting addresses as text from users. You might want to store lldb::addr_t's for Symbol values for space concerns, but presumably the Symbol File would know the relevant address space, so it would fix that up and always hand out Addresses.

This would be a pretty big but mostly formal change. It would seem more natural, Address is our abstraction for addresses in a process. Instead we have this odd mix of API's that take lldb::addr_t and some that take Address.

Jun 18 2020, 3:14 AM · Restricted Project

Jun 8 2020

paolosev added a comment to D80863: [WebAssembly] Eliminate range checks on br_tables.

@paolosev Hmm, I suspect that there is a bad interaction between the code with the range check removed and the algorithm that lays out and structures the basic blocks. I will take a closer look!

Jun 8 2020, 3:33 PM · Restricted Project
paolosev added a comment to D80863: [WebAssembly] Eliminate range checks on br_tables.

I was testing the performance of a program with a big switch statement in a loop, a very common pattern in C/C++ and I came across the problem with needless range checks with br_table that this patch is fixing (Sweet! :-))
However, testing the latest version of Emscripten, with this fix, I am finding that Clang now emits "worse" bytecode for my small test program (attached{F12106893}, compiled with emcc -O3 micro-interp.c -o micro-interp.js).
The code is like:

enum Op {
    A = 0, B, C, D, E, F, G, H, I, J, K, L
};
int f(Op* ops, int len) {
    int result = 0;
    for (int i = 0; i < len; i++) {
        Op op = ops[i];
        switch (op) {
            case A: { ... break; }
            case B: { ... break; }
            case C: { ... break; }
		...
            default: { ... break; }
        }
    }
    return result;
}
Jun 8 2020, 1:02 AM · Restricted Project

May 21 2020

paolosev added a comment to D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.

Hello @clayborg , @labath,
Any thoughts on this latest patch? :-)

May 21 2020, 9:05 PM · Restricted Project

May 15 2020

paolosev added a comment to D79428: [WebAssembly] Added Debug Fixup pass.

I get assertion failures in debug builds:

May 15 2020, 12:52 AM · Restricted Project

May 8 2020

paolosev added a comment to D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.
May 8 2020, 1:34 AM · Restricted Project
paolosev updated the diff for D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.
  • For the DWARF expression evaluation, I am here following the suggestion of defining a Factory plugin (DWARFEvaluatorFactory) which is cached by Module. Is there a way to define a simple plugin function, avoiding all the overhead of a plugin class? Most of the state is in DWARFExpression, which is passed to the evaluator, so the amount of state to pass around is much smaller.
May 8 2020, 1:34 AM · Restricted Project

May 4 2020

paolosev updated the diff for D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.

This patch is a work in progress where I am refactoring the code, as suggested, to remove almost all dependencies on Wasm from the LLDB core that were present in the previous patch. In particular:

May 4 2020, 5:18 AM · Restricted Project

May 1 2020

paolosev added a comment to D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.

The thing I am resisting is to put all of this stuff in the the ProcessGDBRemote class. Instead of trying to generalize it so that it can handle everything (and generalize to the wrong thing), I very much like the idea of introducing a WasmProcess plugin class that handles all wasm stuff. If that class happens to use parts of gdb-remote, then so be it, but it means that it's not a problem for that class to use a dialect of the gdb-remote protocol, which includes as many wasm-specific packets as it needs. Then this class can create it's own implementation of the "Unwind" interface, which will use some WasmProcess-specific apis to undwind, but that will also be ok, since both classes will be wasm-specific.

May 1 2020, 12:09 AM · Restricted Project

Apr 29 2020

paolosev added a comment to D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.

Also, to eliminate qWasmCallStack we could maybe add the call stack (a list of PCs) to the stop packet. The format of a stop packet is:

T AA n1:r1;n2:r2;…
Apr 29 2020, 11:48 AM · Restricted Project
paolosev added a comment to D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.
In D78801#2010536, @ted wrote:

Hi Paulo,
@clayborg asked me to look at this, because I've worked with systems that have multiple address spaces. I was thinking, instead of a WebAssembly specific memory read, we should implement an optional generic memory read and write with memory space support.

So instead of qWasmMem:frame_index;addr;len, we have something like qMemSpaceRead:addr;space;len and qMemSpaceWrite:addr;space;len;data .

"space" is stub dependent - it's just a number, and can mean different things for different targets. It could be different modules, like you talk about here, or different physical memory areas like in a Motorola 56K DSP, or different ways to get at the same memory (physical/virtual/cacheable, or directly controlling MESI bits instead of relying on TLB entries) like I implemented in FSLDBG.

If we do this, other targets that want to work with memory spaces can use them instead of having to implement their own extensions.

About the expression problem - the IR Interpreter doesn't handle some complex expressions. It needs work, but most targets use JIT. On Hexagon, we only recently enabled JIT in the compiler, and haven't done it in the debugger yet, so we use the IR Interpreter for everything. Unfortunately I haven't had time to dive into it to make it better.

Apr 29 2020, 11:16 AM · Restricted Project
paolosev added a comment to D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.

What if the wasm engine actually made some effort to present a more "conventional" view of the wasm "process"? I.e., in addition to the "implied" PC, we could have an "implied" SP (and maybe an FP too). Then it could lay out the call stack and the function arguments in "memory", and point the "SP" to it so that lldb is able to reconstruct the frames&variables using the "normal" algorithm. For the sake of exposition, lets assume the following "encoding" of the 64 bit memory space:

unsigned:16 type; // 0x5555 = stack
unsigned:16 tid; // are there threads in wasm?
unsigned:16 frame; // grows down, 0x0000 = bottommost frame (main), 0x0001 = second from bottom, etc.
unsigned:16 address;

Then the engine could say that the "SP" of thread 0x1234 is 0x5555123400050000, "FP" is `0x5555123400048010 and the have the memory contents be

0x5555123400048020: // third "local" (of frame 4)
0x5555123400048018: // second "local" (of frame 4)
0x5555123400048010: // first "local" (of frame 4)
0x5555123400048008: 0x5555123400038010 // previous FP (frame 3)
0x5555123400048000: ???? // previous PC (frame 3)
0x5555123400040010: // third "argument" (of frame 4)
0x5555123400040008: // second "argument" (of frame 4)
0x5555123400040000: // first "argument" (of frame 4)
0x5555123400038020: // third "local" (of frame 3)
0x5555123400038018: // second "local" (of frame 3)
0x5555123400038010: // first "local" (of frame 3)
0x5555123400038008: 0x5555123400028010 // previous FP (frame 2)
0x5555123400038000: ???? // previous PC (frame 2)
etc.

Then all it would be needed is to translate DW_OP_WASM_location into an appropriate FP+offset combo. Somehow...

I realize that this is basically throwing the problem "over the fence", and asking the other side to deal with things, but I am starting to get sceptical that we will be able to come up with a satisfactory solution within lldb.

Apr 29 2020, 10:11 AM · Restricted Project

Apr 28 2020

paolosev added a comment to D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.

Could we just always use memory reading and have the address contain more info? Right now you have the top 32 bits for the module ID. Could it be something like:

struct WasmAddress {
  uint64_t module_id:16;
  uint64_t space:4; // 0 == code, 1 == data, 2 == global, 3==local, 4 == stack
  uint64_t frame_id:??;
  uint64_t addr: ??;
}

This would be a bitfield that would all fit into a 64 bit value and could then be easily sent to the GDB server with the standard m and M packets.

Apr 28 2020, 6:22 PM · Restricted Project
paolosev added a comment to D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.

Thanks for the explanations if everything WASM related. I now understand much better what you have.

Read Wasm memory.
IN : $qWasmMem:frame_index;addr;len
OUT: $xx..xx

frame index seems weird in a memory read packet. Seems like the module ID should be passed instead of the frame index.

Reading memory could be handled with memory identifiers, or segments. Currently the packets for m and M only have an address, but someone out there must support reading from CODE and DATA address segments in the DSP world. I have an email out to Ted Woodward to see what they do for Qualcomm's hexagon DSPs. I'll let you know what I find. Maybe each WASM module can identify N segments it needs and each module would have its own unique segments. Are module ID's just 1 based indexes?

Apr 28 2020, 5:50 PM · Restricted Project
paolosev added a comment to D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.

So is there memory to be read from the WASM runtime? Couldn't DW_OP_WASM_location 0x0 +8 be turned into an address that can be used to read the variable? It is also unclear what DW_OP_stack_value is used for here. The DWARF expression has no idea how many bytes to read for this value unless each virtual stack location knows how big it is? What happens if you have an array of a million items? That will not fit on the DWARF expression stack and each member would need to be read from memory?

It seems like the DW_OP_WASM_location + args should result in the address of the variable being pushed into the stack and the DW_OP_stack_value should be removed. This would mean at the end of the expression the address of the variable is on the stack and LLDB will just read it using the normal memory read? Am I missing something? Are there multiple memory regions? Are variables not considered to be in memory?

Apr 28 2020, 4:45 PM · Restricted Project

Apr 27 2020

paolosev added a comment to D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.

Thanks for your comments!
I have refactored this code in a separate patch, https://reviews.llvm.org/D78978, removing WasmProcessGDBRemote, moving part of the logic into ProcessGDBRemote but still keeping class UnwindWasm.
Let me know what you think...

Apr 27 2020, 6:52 PM · Restricted Project
paolosev updated the summary of D78978: [LLDB] Add support for WebAssembly debugging.
Apr 27 2020, 6:20 PM · Restricted Project
paolosev created D78978: [LLDB] Add support for WebAssembly debugging.
Apr 27 2020, 6:20 PM · Restricted Project

Apr 26 2020

paolosev updated the diff for D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.

I am adding all the pieces to this patch to make the whole picture clearer; I thought to add a piece at the time to simplify reviews, but probably it ended up making things more obscure. I can always split this patch later and I need to refactor everything anyway.

Apr 26 2020, 11:57 PM · Restricted Project

Apr 24 2020

paolosev added a comment to D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.

What is the best way to test classes WasmProcessGDBRemote and UnwindWasm?

Apr 24 2020, 2:06 AM · Restricted Project
paolosev created D78801: [LLDB] Add class WasmProcess for WebAssembly debugging.
Apr 24 2020, 2:06 AM · Restricted Project

Apr 13 2020

paolosev added a comment to D77353: [WebAssembly] Add DW_OP_WASM_location_int.

@paolosev you may have an opinion on this, given that you're already working with this data as well?

Apr 13 2020, 10:56 PM · debug-info, Restricted Project

Mar 26 2020

paolosev committed rGaff75e1a1fac: [lld][Wasm] Wasm-ld emits invalid .debug_ranges entries for non-live symbols (authored by paolosev).
[lld][Wasm] Wasm-ld emits invalid .debug_ranges entries for non-live symbols
Mar 26 2020, 2:42 PM
paolosev closed D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.
Mar 26 2020, 2:42 PM · Restricted Project, lld
paolosev added a comment to D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.

Regarding the .debug_loc section. I did a quick experiment it looks like, even before this patch we get the expected output:

test.o:	file format WASM

.debug_loc contents:
0x00000000: 
            [0x00000005, 0x00000009): DW_OP_consts +21, DW_OP_stack_value

0x0000001d: 
            [0x0000000a, 0x0000000e): DW_OP_consts +7, DW_OP_stack_value

It looks like reason this works is because there is only one relocation per function here. i.e. there are only two relocations in the above object code. I appears the individual ranges do not themselves contain any relocations.

Hmm, what does llvm-dwarfdump -debug-loc -v print for this? I did implement DWARF loclist base address specifier support which means sometimes it'll use a base address specifier (which would be only one address) & everything would be offsets relative to that - but that's not always used.

Hmm, maybe I'm mistaken... maybe we do /always/ use base address specifiers in debug_loc now... Yep, that's it, always using them.

Sorry for the distraction & thanks for talking it through with me!

Mar 26 2020, 2:42 PM · Restricted Project, lld
paolosev added a comment to D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.

Yes, I managed to get a .debug_loc section also for Wasm with similar code, and debug_loc does not seem to present the same problems with GC'd functions.

Even without your patch? If without your patch you were getting range list entries of zero/zero, I'd expect you to get debug_loc entries with zero/zero too. Any ideas why it doesn't? They should be a similar pair of relocations (though it looks like your fix should work for debug_loc too, which is good).

Looking at the generated DWARF, this patch does not change the offsets for debug_loc. Like in the example above, there are loc entries for variables in the deleted function f2:

0x0000004a:   DW_TAG_subprogram
                DW_AT_low_pc    (0x0000000000000000)
                DW_AT_high_pc   (0x000000000000004d)
                DW_AT_frame_base        (DW_OP_WASM_location 0x0 +1, DW_OP_stack_value)
                DW_AT_GNU_all_call_sites        (true)
                DW_AT_linkage_name      ("_Z2f2Ri")
                DW_AT_name      ("f2")
                DW_AT_decl_file ("C:\dev\llvm-project\release_x64\T\main.cc")
                DW_AT_decl_line (3)
                DW_AT_type      (0x000000cd "int")
                DW_AT_external  (true)
[...]
0x00000071:     DW_TAG_variable
                  DW_AT_location        (0x00000000:
                     [0x00000014, 0x0000001b): DW_OP_consts +42, DW_OP_stack_value
                     [0x00000026, 0x0000002d): DW_OP_consts +21, DW_OP_stack_value
                     [0x0000003f, 0x0000004d): DW_OP_WASM_location 0x0 +2, DW_OP_stack_value)
                  DW_AT_name    ("i")
                  DW_AT_decl_file       ("C:\dev\llvm-project\release_x64\T\main.cc")
                  DW_AT_decl_line       (4)
                  DW_AT_type    (0x000000cd "int")

The entries seems to assume that the deleted function starts at offset 0, and ends at 0x4d, and this is true also without the patch.

Mar 26 2020, 2:42 PM · Restricted Project, lld

Mar 24 2020

paolosev updated the diff for D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.
Mar 24 2020, 8:24 PM · Restricted Project, lld
paolosev added a comment to D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.

@sbc100 Adding a test.

@dblaikie Strangely, I can't find a way to generate a .debug_loc section for Wasm.

Maybe it's not applicable to wasm, I don't know much about it - but if it optimizes variables into registers (or out of them), then it shuold be producible.

Here's a simple test case I use for debug_loc in C++ on a silicon target:

void f1();
int main() {
  int i = 3;
  f1();
  i = 7;
  f1();
}

Compiled with optimizations enabled, the storage for 'i' will be eliminated and a DWARF location list will be used to describe 'i' as having the value 3 over the first call instruction, and the value 7 over the second call instruction.

Mar 24 2020, 8:24 PM · Restricted Project, lld

Mar 23 2020

paolosev updated the diff for D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.

@sbc100 Adding a test.

Mar 23 2020, 8:41 PM · Restricted Project, lld

Mar 21 2020

paolosev updated the diff for D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.

Thank you Sam, fixing this in calcNewValue also works and it's cleaner.

Mar 21 2020, 10:59 PM · Restricted Project, lld

Feb 27 2020

paolosev committed rG256e61699b19: [LLDB] Fix AddressSanitizer failure in MemoryCache (authored by paolosev).
[LLDB] Fix AddressSanitizer failure in MemoryCache
Feb 27 2020, 11:36 AM
paolosev closed D75200: [LLDB] AddressSanitizer failure in MemoryCache.
Feb 27 2020, 11:36 AM · Restricted Project
paolosev retitled D75200: [LLDB] AddressSanitizer failure in MemoryCache from AddressSanitizer failure in MemoryCache to [LLDB] AddressSanitizer failure in MemoryCache.
Feb 27 2020, 11:35 AM · Restricted Project

Feb 26 2020

paolosev added a comment to D75200: [LLDB] AddressSanitizer failure in MemoryCache.

The Harbormaster errors don't seem to be real failures:

/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/lldb/source/Core/Module.cpp:9:10: error: 'lldb/Core/Module.h' file not found [clang-diagnostic-error]
#include "lldb/Core/Module.h"
         ^
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/lldb/source/Target/Memory.cpp:9:10: error: 'lldb/Target/Memory.h' file not found [clang-diagnostic-error]
#include "lldb/Target/Memory.h"
         ^
Feb 26 2020, 11:06 PM · Restricted Project
paolosev updated the diff for D75200: [LLDB] AddressSanitizer failure in MemoryCache.

Not sure this is the right fix. The calling code should listen to how many bytes were actually read from memory and avoid touching any bytes. So we should fix Module::GetMemoryObjectFile to resize its buffer back to only the size that was read. S

Feb 26 2020, 6:34 PM · Restricted Project
paolosev added a comment to D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

Who is the owner of this MemoryCache code I should ask for a review?

It sounds like the fix will be fairly straightforward. I think anyone on this thread should be able to review that.

Feb 26 2020, 11:14 AM · Restricted Project
paolosev updated the summary of D75200: [LLDB] AddressSanitizer failure in MemoryCache.
Feb 26 2020, 11:05 AM · Restricted Project
paolosev created D75200: [LLDB] AddressSanitizer failure in MemoryCache.
Feb 26 2020, 11:04 AM · Restricted Project

Feb 25 2020

paolosev added a comment to D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

Hi @paolosev, the lldb sanitized bot is flagging a container-overflow error here. I know that this /can/ have FPs when sanitized and unsanitized code is mixed, but we should be in purely sanitized code here, and this looks like a valid report. PTAL.

Feb 25 2020, 6:37 PM · Restricted Project
paolosev added a comment to D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.
In D72751#1892514, @vsk wrote:

Hi @paolosev, the lldb sanitized bot is flagging a container-overflow error here. I know that this /can/ have FPs when sanitized and unsanitized code is mixed, but we should be in purely sanitized code here, and this looks like a valid report. PTAL.

http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake-sanitized/992/testReport/junit/lldb-api/functionalities_gdb_remote_client/TestWasm_py/

Feb 25 2020, 4:57 PM · Restricted Project

Feb 18 2020

paolosev accepted D74780: [lldb/Plugin] Reject WASM and Hexagon in DynamicLoaderStatic.

Sorting the plugins so the static loader is initialized before the WASM loader should exercise this code path.

Feb 18 2020, 12:45 PM · Restricted Project
paolosev created D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.
Feb 18 2020, 11:21 AM · Restricted Project, lld

Feb 17 2020

paolosev added a comment to D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

I think it depends on whether the DynamicLoaderStatic should be a fallback. If it doesn't make sense then yes, I think we should reject that triple there.

It should not be a fallback. Ok! I'll create a new patch with this change.

Feb 17 2020, 4:16 PM · Restricted Project
paolosev added a comment to D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

@JDevlieghere added inline comments:

lldb/source/API/SystemInitializerFull.cpp 244
What's the rationale here? Plugins shouldn't rely on the order in which they are initialized. This breaks when the initializers are auto generated. Can we remove this dependency?

Feb 17 2020, 3:40 PM · Restricted Project

Feb 14 2020

paolosev added a comment to D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

yeah, go for it.

Feb 14 2020, 4:03 PM · Restricted Project

Feb 12 2020

paolosev updated the diff for D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

Fix patch after rebasing:

  • Use macros to initialize plugins in SystemInitializer.
  • Move tests from /packages/Python/lldbsuite/test/functionalities/gdb_remote_client/ to /test/API/functionalities/gdb_remote_client/
Feb 12 2020, 4:13 PM · Restricted Project

Feb 7 2020

paolosev updated the diff for D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

Ok, let's give this one more try. I have a couple of inline comments for the further simplification of the test case.

Feb 7 2020, 1:03 PM · Restricted Project

Feb 6 2020

paolosev requested review of D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.
Feb 6 2020, 4:35 PM · Restricted Project
paolosev reopened D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

As promised, here are the comments on the new tests. I think that most of the py3 incompatibilities will go away once we get rid of the yaml preprocessing step, but it would be good to verify this with python 3 nonetheless...

Feb 6 2020, 4:35 PM · Restricted Project
paolosev updated the diff for D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

Modified tests to be compatible with Python3.

Feb 6 2020, 4:32 PM · Restricted Project

Feb 5 2020

paolosev added a comment to D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

There is Windows Build Bot failure http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/13427. Can you please fix or revert it?

Cannot open include file: 'Plugins/DynamicLoader/wasm-DYLD/DynamicLoaderWasmDYLD.h': No such file or directory
Feb 5 2020, 4:56 PM · Restricted Project
paolosev added a comment to D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

Thank you Derek, Jonas; I am sorry for all the trouble...

Feb 5 2020, 4:10 PM · Restricted Project
paolosev updated the diff for D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

Fixing ObjectFile/wasm tests.

Feb 5 2020, 1:13 PM · Restricted Project

Feb 4 2020

paolosev updated the diff for D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

Rebasing.

Feb 4 2020, 6:40 PM · Restricted Project

Jan 31 2020

paolosev updated the diff for D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

Fixed the tests as suggested, and also added a couple more tests, to cover both the case where the Wasm module is loaded from memory and the case where it is loaded from a file.

Jan 31 2020, 3:03 PM · Restricted Project

Jan 29 2020

paolosev added a comment to D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.
  • make the base DynamicLoader class instantiatable, and use it whenever we fail to find a specialized plugin
  • same as above, but only do that for ProcessGDBRemote instances
  • make ProcessGDBRemote call LoadModules() itself if no dynamic loader instance is available WDYT?

I am fine with 1 as long as we document the DynamicLoader class to say that it will call Process::LoadModules() and will be used if no specialized loader is needed for your platform. I would like to a see a solution that will work for any process plug-in and not just ProcessGDBRemote. If we change solution 3 above to say "Make lldb_private::Process call LoadModules() itself if no dynamic loader instance is available" then solution 3 is also fine.

Jan 29 2020, 4:42 PM · Restricted Project

Jan 27 2020

paolosev added a comment to D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

Comments inline...

Jan 27 2020, 4:42 PM · Restricted Project
paolosev updated the diff for D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

Thanks. I am glad that we were able to sort that out.

Now that there's nothing wasm-specific in DynamicLoaderWasmDYLD::LoadModuleAtAddress, I have another question. :)

The code in that function is a subset of the base DynamicLoader::LoadModuleAtAddress method you are overriding. Is there a reason for that?
It doesn't seem like the additional stuff in the base method should hurt here. What the additional code does is that it tries to search for the object file on the local filesystem, and if it finds it, it will use that instead of copying the file over from the remote. In fact, that sounds like it could be useful here too, as copying that much data over gdb-remote isn't particularly fast..

What do you think about that?

Jan 27 2020, 4:06 PM · Restricted Project

Jan 24 2020

paolosev updated the diff for D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

Modified to set m_file_offset to be the correct offset of the Code section. This also simplifies the code in ObjectFileWasm to avoid a special case for the code section in ObjectFileWasm::SetLoadAddress.
The DWARF code seems to only use GetFileAddress(), which still needs to return zero for the Code section.

Jan 24 2020, 12:07 PM · Restricted Project

Jan 23 2020

paolosev updated the diff for D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

Thanks for the explanation! I wasn't quite clear on "executable module" here, but after your comments I realized that Target::SetExecutableModule() should not probably be called also for Wasm modules.
The point is that ObjectFileWasm::CalculateType() should return eTypeSharedLibrary, not eTypeExecutable.
With this change the first issue is easily solved: we just need to call Target::SetSectionLoadAddress() once, in ObjectFileWasm::SetLoadAddress() because Target::SetExecutableModule() -> Target::ClearModules() -> SectionLoadList::Clear() is not called, and DynamicLoaderWasmDYLD::DidAttach() can be simplified to just call ProcessGDBRemote::LoadModules().
Does this solution work for you? If so, we should look at the second point, the need to initialize m_file_addr = m_file_offset = 0 for the "code" Section in order to make the DWARF symbols work...

Jan 23 2020, 6:23 PM · Restricted Project

Jan 22 2020

paolosev updated the diff for D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

I have verified the logic of the dynamic loader quite carefully, but there are a couple of things to clarify.

Jan 22 2020, 11:54 PM · Restricted Project

Jan 15 2020

paolosev added a comment to D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.

BTW, I had to fix this patch (cd9e5c32302cd3b34b796683eedb072c6a1cfdc1) to build on macOS. uint64_t and size_t are differently spelled (though I think otherwise equivalent.) One is "long long unsigned int", the other "long unsigned int". I have no idea why that's true, but std::min refuses to compare a size_t and a unit64_t. Anyway, I fixed this by casting one of the two sides of the comparison. But this was causing problems because we have an api (ReadImageData) that takes a uint64_t for the offset and a size_t for the size. That seems a little weird to me, why are these different types?

Jan 15 2020, 6:52 PM · Restricted Project
paolosev updated the diff for D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging.

Modified test to have two "inlined" yaml files and use "yaml2obj --docnum".

Jan 15 2020, 1:02 PM · Restricted Project
paolosev added a comment to D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.

Thanks. I think this is looking very good now. Excited to have this ready.

Do you have commit access?

Jan 15 2020, 10:22 AM · Restricted Project

Jan 14 2020

paolosev created D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.
Jan 14 2020, 11:40 PM · Restricted Project
paolosev added a comment to D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.

What is the best way to test this class?

Jan 14 2020, 11:40 PM · Restricted Project
paolosev updated the diff for D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging.

Rebasing from D71575.

Jan 14 2020, 11:03 PM · Restricted Project
paolosev added inline comments to D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.
Jan 14 2020, 10:43 PM · Restricted Project
paolosev updated the diff for D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.
Jan 14 2020, 10:34 PM · Restricted Project
paolosev added a comment to D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging.

The patch looks pretty good. A reasonable way to test this would be again via lldb-test object-file . The command dumps the "unified section list" of the module, so if the debug info sections show up there, you know the symbol vendor has done it's job. You can look at test/Shell/ObjectFile/ELF/build-id-case.yaml for inspiration.

Jan 14 2020, 4:02 PM · Restricted Project
paolosev updated the diff for D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging.

Added "lldb-test object-file" test.

Jan 14 2020, 3:53 PM · Restricted Project

Jan 13 2020

paolosev created D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging.
Jan 13 2020, 2:21 PM · Restricted Project

Jan 10 2020

paolosev abandoned D71788: [llvm-strip] Add WebAssembly support.
Jan 10 2020, 3:39 PM · Restricted Project
paolosev added a comment to D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.

I apologize for the noob question, but how do I schedule a build for this diff with Harbormaster?

Jan 10 2020, 10:41 AM · Restricted Project
paolosev added inline comments to D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.
Jan 10 2020, 10:03 AM · Restricted Project

Jan 8 2020

paolosev updated the diff for D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.
Jan 8 2020, 11:08 AM · Restricted Project
paolosev added inline comments to D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.
Jan 8 2020, 11:08 AM · Restricted Project

Jan 5 2020

paolosev added inline comments to D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.
Jan 5 2020, 5:43 AM · Restricted Project
paolosev updated the diff for D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.

Addressed more review comments:

  • removed code to manage "external_debug_info" sections; logic for this will be implemented in the symbol vendor code.
  • modified test code, from unittests to be Shell "lit" tests.
Jan 5 2020, 5:43 AM · Restricted Project

Jan 3 2020

paolosev added a comment to D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.

A bunch more comments from me. :)

A higher level question I have is whether there's something suitable within llvm for parsing wasm object files that could be reused. I know this can be tricky with files loaded from memory etc., so it's fine if it isn't possible to do that, but I am wondering if you have considered this option.

Jan 3 2020, 10:43 AM · Restricted Project

Dec 20 2019

paolosev added a comment to D71788: [llvm-strip] Add WebAssembly support.

Doh! I didn't know there was this work ongoing.
I think we could maybe continue from there, once https://reviews.llvm.org/D70930 is merged; what I would like is to have something like --split-dwo that works with WASM files and only extracts DWARF sections.

Dec 20 2019, 6:12 PM · Restricted Project
paolosev created D71788: [llvm-strip] Add WebAssembly support.
Dec 20 2019, 3:09 PM · Restricted Project
paolosev added inline comments to D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.
Dec 20 2019, 10:09 AM · Restricted Project
paolosev updated the diff for D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.
Dec 20 2019, 10:09 AM · Restricted Project

Dec 18 2019

paolosev updated the diff for D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.
Dec 18 2019, 12:54 AM · Restricted Project
paolosev updated the diff for D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.
Dec 18 2019, 12:45 AM · Restricted Project

Dec 17 2019

paolosev updated the diff for D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.

Addressed first review comments.

Dec 17 2019, 10:12 PM · Restricted Project
paolosev added a comment to D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.

Thanks for all the comments! I am updating the code following your suggestions.
Next step will be to split this into three distinct patches, as suggested by Pavel.

Dec 17 2019, 10:12 PM · Restricted Project

Dec 16 2019

paolosev updated the summary of D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.
Dec 16 2019, 3:21 PM · Restricted Project
paolosev created D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging.
Dec 16 2019, 3:18 PM · Restricted Project