Page MenuHomePhabricator

paolosev (Paolo Severini)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 12 2019, 11:43 AM (16 w, 3 d)

Recent Activity

Thu, Mar 26

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
Thu, Mar 26, 2:42 PM
paolosev closed D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.
Thu, Mar 26, 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!

Thu, Mar 26, 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.

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

Tue, Mar 24

paolosev updated the diff for D74781: Wasm-ld emits invalid .debug_ranges entries for non-live symbols.
Tue, Mar 24, 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.

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

Mon, Mar 23

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

@sbc100 Adding a test.

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

Sat, Mar 21

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.

Sat, Mar 21, 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 added a child revision for D72650: [LLDB] Add SymbolVendorWasm plugin for WebAssembly debugging: D72751: [LLDB] Add DynamicLoaderWasmDYLD plugin for WebAssembly debugging.
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
paolosev added a child revision for D71575: [LLDB] Add ObjectFileWasm plugin for WebAssembly debugging: 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