Page MenuHomePhabricator

mstorsjo (Martin Storsjö)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 25 2016, 12:54 PM (164 w, 6 d)

Recent Activity

Today

mstorsjo added a comment to D67885: [LLDB] Add a missing specification of linking against dbghelp.

Just as a drive by comment, there is code in DebugInfo/Symbolize/Symbolize.cpp that loads dbghelp via #pragma comment(lib... trick

Mon, Sep 23, 4:25 AM · Restricted Project
mstorsjo added a reviewer for D67892: [LLDB] Fix typo in RegisterContextDarwin_arm64: labath.
Mon, Sep 23, 3:24 AM · Restricted Project
mstorsjo added a reviewer for D67893: [LLDB] Check for _WIN32 instead of _MSC_VER for code specific to windows in general: labath.
Mon, Sep 23, 3:24 AM · Restricted Project
mstorsjo added a reviewer for D67894: [LLDB] Rework a MinGW build fix from D65691: labath.
Mon, Sep 23, 3:24 AM · Restricted Project
mstorsjo added a reviewer for D67895: [LLDB] Avoid a warning about an unused static variable: labath.
Mon, Sep 23, 3:23 AM · Restricted Project
mstorsjo updated the diff for D67861: [LLDB] Remove a now redundant windows specific workaround.

Updated the patch to simply remove the now redundant workaround.

Mon, Sep 23, 3:23 AM · Restricted Project
mstorsjo added a reviewer for D67896: [LLDB] Add a void* cast when passing object pointers to printf %p: labath.
Mon, Sep 23, 3:23 AM · Restricted Project
mstorsjo added a comment to D67861: [LLDB] Remove a now redundant windows specific workaround.

Judging by the microsoft docs, vsnprintf is avaliable at least since VS 2017 (the lowest version supported by llvm). Is it possible that we can just delete this #ifdef ?

Mon, Sep 23, 3:14 AM · Restricted Project
mstorsjo added inline comments to D67851: llvm-undname: Add support for demangling typeinfo names.
Mon, Sep 23, 1:45 AM · Restricted Project

Yesterday

mstorsjo created D67896: [LLDB] Add a void* cast when passing object pointers to printf %p.
Sun, Sep 22, 12:24 PM · Restricted Project
mstorsjo created D67895: [LLDB] Avoid a warning about an unused static variable.
Sun, Sep 22, 12:19 PM · Restricted Project
mstorsjo created D67894: [LLDB] Rework a MinGW build fix from D65691.
Sun, Sep 22, 12:16 PM · Restricted Project
mstorsjo created D67893: [LLDB] Check for _WIN32 instead of _MSC_VER for code specific to windows in general.
Sun, Sep 22, 12:14 PM · Restricted Project
mstorsjo created D67892: [LLDB] Fix typo in RegisterContextDarwin_arm64.
Sun, Sep 22, 12:07 PM · Restricted Project
mstorsjo added a comment to D67885: [LLDB] Add a missing specification of linking against dbghelp.

@hhb - This is the only change left for builds of lldb on mingw to succeed for me (for x86), but I have a whole bunch of more changes lined up that fix warnings and other issues I've found.

Sun, Sep 22, 11:35 AM · Restricted Project
mstorsjo abandoned D67857: [LLDB] Include lldb/Host/windows/windows.h on any windows target.

D67887 was committed with the same change.

Sun, Sep 22, 11:08 AM · Restricted Project
mstorsjo added a comment to D67887: Use _WIN32 instead of _MSC_VER.

Just FTR, this was identical to D67857 that I posted a few days ago.

Sun, Sep 22, 11:08 AM · Restricted Project, Restricted Project

Sat, Sep 21

mstorsjo created D67885: [LLDB] Add a missing specification of linking against dbghelp.
Sat, Sep 21, 2:23 PM · Restricted Project
mstorsjo committed rG5534a6750087: [LLDB] Cast -1 (as invalid socket) to the socket type before comparing (authored by mstorsjo).
[LLDB] Cast -1 (as invalid socket) to the socket type before comparing
Sat, Sep 21, 12:10 PM
mstorsjo committed rGed78dc8e4371: [LLDB] Use SetErrorStringWithFormatv for cases that use LLVM style format… (authored by mstorsjo).
[LLDB] Use SetErrorStringWithFormatv for cases that use LLVM style format…
Sat, Sep 21, 12:10 PM
mstorsjo committed rG5c38730dbd00: [LLDB] Use LLVM_FALLTHROUGH instead of a custom comment (authored by mstorsjo).
[LLDB] Use LLVM_FALLTHROUGH instead of a custom comment
Sat, Sep 21, 12:10 PM
mstorsjo committed rG2e25c44dc3fa: [LLDB] Check for the GCC/MinGW compatible arch defines for windows, in addition… (authored by mstorsjo).
[LLDB] Check for the GCC/MinGW compatible arch defines for windows, in addition…
Sat, Sep 21, 12:09 PM
mstorsjo committed rGf4deacf995c7: [LLDB] Fix compilation for MinGW, remove redundant class name on inline member (authored by mstorsjo).
[LLDB] Fix compilation for MinGW, remove redundant class name on inline member
Sat, Sep 21, 12:09 PM
mstorsjo committed rG1bfdab52a76b: [CodeView] Add pragma push/pop_macro for ARM64_FPSR to enum header (authored by mstorsjo).
[CodeView] Add pragma push/pop_macro for ARM64_FPSR to enum header
Sat, Sep 21, 12:09 PM
mstorsjo committed rL372486: [LLDB] Cast -1 (as invalid socket) to the socket type before comparing.
[LLDB] Cast -1 (as invalid socket) to the socket type before comparing
Sat, Sep 21, 12:09 PM
mstorsjo closed D67863: [LLDB] Cast -1 (as invalid socket) to the socket type before comparing.
Sat, Sep 21, 12:09 PM · Restricted Project, Restricted Project
mstorsjo committed rL372485: [LLDB] Use SetErrorStringWithFormatv for cases that use LLVM style format….
[LLDB] Use SetErrorStringWithFormatv for cases that use LLVM style format…
Sat, Sep 21, 12:09 PM
mstorsjo committed rL372484: [LLDB] Use LLVM_FALLTHROUGH instead of a custom comment.
[LLDB] Use LLVM_FALLTHROUGH instead of a custom comment
Sat, Sep 21, 12:09 PM
mstorsjo closed D67862: [LLDB] Use SetErrorStringWithFormatv for cases that use LLVM style format strings.
Sat, Sep 21, 12:09 PM · Restricted Project, Restricted Project
mstorsjo closed D67860: [LLDB] Use LLVM_FALLTHROUGH instead of a custom comment.
Sat, Sep 21, 12:09 PM · Restricted Project, Restricted Project
mstorsjo committed rL372483: [LLDB] Check for the GCC/MinGW compatible arch defines for windows, in addition….
[LLDB] Check for the GCC/MinGW compatible arch defines for windows, in addition…
Sat, Sep 21, 12:09 PM
mstorsjo committed rL372482: [LLDB] Fix compilation for MinGW, remove redundant class name on inline member.
[LLDB] Fix compilation for MinGW, remove redundant class name on inline member
Sat, Sep 21, 12:09 PM
mstorsjo closed D67858: [LLDB] Check for the GCC/MinGW compatible arch defines for windows, in addition to MSVC defines.
Sat, Sep 21, 12:08 PM · Restricted Project, Restricted Project
mstorsjo closed D67856: [LLDB] Fix compilation for MinGW, remove redundant class name on inline member.
Sat, Sep 21, 12:08 PM · Restricted Project, Restricted Project
mstorsjo committed rL372481: [CodeView] Add pragma push/pop_macro for ARM64_FPSR to enum header.
[CodeView] Add pragma push/pop_macro for ARM64_FPSR to enum header
Sat, Sep 21, 12:08 PM
mstorsjo closed D67864: [CodeView] Add pragma push/pop_macro for ARM64_FPSR to enum header.
Sat, Sep 21, 12:08 PM · Restricted Project
mstorsjo added a reviewer for D67861: [LLDB] Remove a now redundant windows specific workaround: compnerd.
Sat, Sep 21, 11:02 AM · Restricted Project
mstorsjo added a reviewer for D67859: [LLDB] Use the Windows SOCKET type on all windows targets, not only MSVC: compnerd.
Sat, Sep 21, 11:01 AM · Restricted Project
mstorsjo added a reviewer for D67857: [LLDB] Include lldb/Host/windows/windows.h on any windows target: compnerd.
Sat, Sep 21, 11:01 AM · Restricted Project

Fri, Sep 20

mstorsjo updated the diff for D67863: [LLDB] Cast -1 (as invalid socket) to the socket type before comparing.

Use static_cast.

Fri, Sep 20, 2:55 PM · Restricted Project, Restricted Project
mstorsjo updated the diff for D67864: [CodeView] Add pragma push/pop_macro for ARM64_FPSR to enum header.

Added a comment.

Fri, Sep 20, 2:28 PM · Restricted Project
mstorsjo added a comment to D67864: [CodeView] Add pragma push/pop_macro for ARM64_FPSR to enum header.

Thanks for catching this. Could the #pragma push and #undef be moved to the line right before CV_REGISTER(ARM64_FPSR, 220).

Fri, Sep 20, 2:21 PM · Restricted Project
mstorsjo created D67864: [CodeView] Add pragma push/pop_macro for ARM64_FPSR to enum header.
Fri, Sep 20, 1:54 PM · Restricted Project
mstorsjo created D67863: [LLDB] Cast -1 (as invalid socket) to the socket type before comparing.
Fri, Sep 20, 1:53 PM · Restricted Project, Restricted Project
mstorsjo created D67862: [LLDB] Use SetErrorStringWithFormatv for cases that use LLVM style format strings.
Fri, Sep 20, 1:53 PM · Restricted Project, Restricted Project
mstorsjo created D67861: [LLDB] Remove a now redundant windows specific workaround.
Fri, Sep 20, 1:45 PM · Restricted Project
mstorsjo created D67860: [LLDB] Use LLVM_FALLTHROUGH instead of a custom comment.
Fri, Sep 20, 1:44 PM · Restricted Project, Restricted Project
mstorsjo created D67859: [LLDB] Use the Windows SOCKET type on all windows targets, not only MSVC.
Fri, Sep 20, 1:34 PM · Restricted Project
mstorsjo created D67858: [LLDB] Check for the GCC/MinGW compatible arch defines for windows, in addition to MSVC defines.
Fri, Sep 20, 1:25 PM · Restricted Project, Restricted Project
mstorsjo added a comment to D67856: [LLDB] Fix compilation for MinGW, remove redundant class name on inline member.

If this doesn't break clang and gcc, fine.

Fri, Sep 20, 1:25 PM · Restricted Project, Restricted Project
mstorsjo created D67857: [LLDB] Include lldb/Host/windows/windows.h on any windows target.
Fri, Sep 20, 1:18 PM · Restricted Project
mstorsjo created D67856: [LLDB] Fix compilation for MinGW, remove redundant class name on inline member.
Fri, Sep 20, 1:09 PM · Restricted Project, Restricted Project
mstorsjo updated the diff for D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode.

Updated the CUDA test based on the suggestion.

Fri, Sep 20, 12:14 PM · Restricted Project
mstorsjo added a comment to D67301: [LLD] Use the unified llvm demangle frontend function. NFC..
In D67301#1665171, @rnk wrote:

I hope at least for ELF, you can keep the "_Z" prefix check. It doesn't need the ObjC "___Z" extension.

Are you sure? Surely users could use -fblocks to get those manglings, or use ObjC on Linux.

We haven't even heard people complaining about lack of demangling of __Z, ___Z or ___Z prefixed names. I still lean towards keeping the "_Z" check for ELF to avoid false positive.

@rnk - do you want to follow up on this further, or should we move forward with this in this form (which is pretty much NFC now).

Fri, Sep 20, 12:14 PM · Restricted Project

Wed, Sep 18

mstorsjo added reviewers for D66889: [libunwind] Fix unw_get_proc_info sometimes returning stale data: mclow.lists, ldionne, EricWF.

Can the runtimes repo maintainers @mclow.lists @ldionne @EricWF give this an ack aswell?

Wed, Sep 18, 9:57 AM

Sun, Sep 15

mstorsjo set the repository for D66889: [libunwind] Fix unw_get_proc_info sometimes returning stale data to rUNW libunwind.
Sun, Sep 15, 10:33 AM
mstorsjo added reviewers for D66889: [libunwind] Fix unw_get_proc_info sometimes returning stale data: compnerd, joerg.
Sun, Sep 15, 8:30 AM

Fri, Sep 13

mstorsjo added inline comments to D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode.
Fri, Sep 13, 9:26 AM · Restricted Project
mstorsjo added a comment to D67053: [LLD] [COFF] Resolve source locations for undefined references using dwarf.

Ping @ruiu

Fri, Sep 13, 3:45 AM · Restricted Project
mstorsjo added inline comments to D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode.
Fri, Sep 13, 3:37 AM · Restricted Project
mstorsjo updated the diff for D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode.

Adapted based on the feedback so far, suggestions on naming and grouping the warning are welcome.

Fri, Sep 13, 3:37 AM · Restricted Project
mstorsjo added a comment to D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode.

OK. If this is causing compatibility pain in practice, following GCC here seems reasonable. We should at least produce a warning on gnu_inline without extern in C++ mode, though.

Fri, Sep 13, 12:37 AM · Restricted Project

Thu, Sep 12

mstorsjo added a comment to D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode.

Seems very surprising to me that the gnu_inline attribute has different behavior in their C and C++ frontends. That said, it looks like it's a behavior that they document; their documentation says "In C++, this attribute does not depend on extern in any way, but it still requires the inline keyword to enable its special behavior." and that matches their observed behavior. And they behave this way even for functions in extern "C" blocks. The question for us, then, is do we want to be compatible with C source code built as C++ (that is, the status quo) and C++ source code using this attribute and targeting older versions of Clang, or with g++?

Thu, Sep 12, 2:12 PM · Restricted Project
mstorsjo added inline comments to D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode.
Thu, Sep 12, 1:48 PM · Restricted Project
mstorsjo added a reviewer for D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode: nickdesaulniers.
Thu, Sep 12, 12:53 PM · Restricted Project
mstorsjo added a comment to D67301: [LLD] Use the unified llvm demangle frontend function. NFC..
In D67301#1665171, @rnk wrote:

I hope at least for ELF, you can keep the "_Z" prefix check. It doesn't need the ObjC "___Z" extension.

Are you sure? Surely users could use -fblocks to get those manglings, or use ObjC on Linux.

We haven't even heard people complaining about lack of demangling of __Z, ___Z or ___Z prefixed names. I still lean towards keeping the "_Z" check for ELF to avoid false positive.

Thu, Sep 12, 12:45 PM · Restricted Project

Wed, Sep 11

mstorsjo updated the diff for D67301: [LLD] Use the unified llvm demangle frontend function. NFC..

Tweaked the syntax for parameter literal names in comments, reinstated a check for config->demangle in the wasm backend.

Wed, Sep 11, 12:30 PM · Restricted Project
mstorsjo added inline comments to D67301: [LLD] Use the unified llvm demangle frontend function. NFC..
Wed, Sep 11, 12:30 PM · Restricted Project
mstorsjo added inline comments to D67301: [LLD] Use the unified llvm demangle frontend function. NFC..
Wed, Sep 11, 10:38 AM · Restricted Project

Tue, Sep 10

mstorsjo updated the diff for D67301: [LLD] Use the unified llvm demangle frontend function. NFC..

Removed use of llvm::Optional in the demangle library, adapted COFF/Symbols.cpp to do a check for "demangled != demangleInput" instead of checking an Optional return value.

Tue, Sep 10, 11:53 PM · Restricted Project
mstorsjo added a comment to D67053: [LLD] [COFF] Resolve source locations for undefined references using dwarf.

Ping @ruiu

Tue, Sep 10, 11:04 PM · Restricted Project
mstorsjo added inline comments to D67301: [LLD] Use the unified llvm demangle frontend function. NFC..
Tue, Sep 10, 9:15 PM · Restricted Project
mstorsjo created D67414: [AST] Treat "inline gnu_inline" the same way as "extern inline gnu_inline" in C++ mode.
Tue, Sep 10, 1:54 PM · Restricted Project
mstorsjo updated the diff for D67301: [LLD] Use the unified llvm demangle frontend function. NFC..

Extended the llvm demangle function to take an optional parameter for requesting it to only demangle symbols with a stricter prefix check (only allowing _Z).

Tue, Sep 10, 12:27 PM · Restricted Project
mstorsjo added a comment to D67301: [LLD] Use the unified llvm demangle frontend function. NFC..

This change is not NFC. This will cause every port of LLD to demangle more names than which were allowed before.

I hope at least for ELF, you can keep the "_Z" prefix check. It doesn't need the ObjC "___Z" extension.

Tue, Sep 10, 12:24 PM · Restricted Project
mstorsjo updated the diff for D67301: [LLD] Use the unified llvm demangle frontend function. NFC..

Updated to use the already unified llvm demangler frontend function, removing a lot of duplicated code in lld.

Tue, Sep 10, 6:41 AM · Restricted Project
mstorsjo added a comment to D67301: [LLD] Use the unified llvm demangle frontend function. NFC..

It turns out most of this discussion already is moot; the llvm demangle library already has a common unified function, which takes itanium names with 1-4 leading underscores. That makes this patch even simpler, mostly just rip out a lot of duplicated code from lld. This makes things a bit looser (the previous lld frontend to itaniumDemangle required it to start with exactly _Z), but as this is the current behaviour of the common demangle library, that's probably not an issue in practice.

Tue, Sep 10, 6:34 AM · Restricted Project
mstorsjo committed rG5d2695903952: [Object] Implement relocation resolver for COFF ARM/ARM64 (authored by mstorsjo).
[Object] Implement relocation resolver for COFF ARM/ARM64
Tue, Sep 10, 5:37 AM
mstorsjo committed rL371515: [Object] Implement relocation resolver for COFF ARM/ARM64.
[Object] Implement relocation resolver for COFF ARM/ARM64
Tue, Sep 10, 5:30 AM
mstorsjo closed D67340: [Object] Implement relocation resolver for COFF ARM/ARM64.
Tue, Sep 10, 5:30 AM · Restricted Project

Mon, Sep 9

mstorsjo updated the diff for D67053: [LLD] [COFF] Resolve source locations for undefined references using dwarf.

Renamed the existing functions to clarify that they operate on codeview debug info only.

Mon, Sep 9, 11:47 PM · Restricted Project
mstorsjo added inline comments to D67053: [LLD] [COFF] Resolve source locations for undefined references using dwarf.
Mon, Sep 9, 1:20 PM · Restricted Project
mstorsjo added inline comments to D67053: [LLD] [COFF] Resolve source locations for undefined references using dwarf.
Mon, Sep 9, 11:48 AM · Restricted Project
mstorsjo added inline comments to D67343: [DebugInfo] Change object::RelocationResolver to return Expected<uint64_t>.
Mon, Sep 9, 2:41 AM · Restricted Project
mstorsjo added inline comments to D67340: [Object] Implement relocation resolver for COFF ARM/ARM64.
Mon, Sep 9, 1:24 AM · Restricted Project
mstorsjo created D67340: [Object] Implement relocation resolver for COFF ARM/ARM64.
Mon, Sep 9, 12:39 AM · Restricted Project
mstorsjo updated the diff for D67053: [LLD] [COFF] Resolve source locations for undefined references using dwarf.

Reduced the test case further, converted it to use spaces instead of tabs. (A few manual edits had given some parts get mixed indentation in the middle of lines before.) Keeping the symbolizer around in the global config object (without std::unique_ptr, as it's allocated by the arena allocator with make<LLVMSymbolizer>()).

Mon, Sep 9, 12:19 AM · Restricted Project

Sun, Sep 8

mstorsjo added a comment to D67301: [LLD] Use the unified llvm demangle frontend function. NFC..

But if we make the demangler implicitly handle both cases, it can also accidentally e.g. try to demangle any symbol which just starts with a capital Z, without any real underscore prefix, on i386 in COFF or in MachO. (I realize the current code in LLD I just committed also has this issue though.)

Sun, Sep 8, 11:22 PM · Restricted Project
mstorsjo added a comment to D67053: [LLD] [COFF] Resolve source locations for undefined references using dwarf.

Do you have an opinion on whether I should keep the LLVMSymbolizer object around between calls?

This is the error path and we don't expect this code to be called many times so its contribution to the overall time may be negligible. Keep the same LLVMSymbolizer will benefit when you symbolize the same object multiple times. If reusing LLVMSymbolizer does not take too many lines, you can do that.

Sun, Sep 8, 11:20 PM · Restricted Project

Sat, Sep 7

mstorsjo added a comment to D67053: [LLD] [COFF] Resolve source locations for undefined references using dwarf.

Reduced the size of the test case by manually cutting out unneeded parts, similar to some of the ELF tests.

The updated test looks good now. Have you had success with larger executables now?

Sat, Sep 7, 11:56 PM · Restricted Project
mstorsjo updated the diff for D67053: [LLD] [COFF] Resolve source locations for undefined references using dwarf.

Reduced the size of the test case by manually cutting out unneeded parts, similar to some of the ELF tests.

Sat, Sep 7, 2:07 PM · Restricted Project
mstorsjo added inline comments to D67053: [LLD] [COFF] Resolve source locations for undefined references using dwarf.
Sat, Sep 7, 2:06 PM · Restricted Project
mstorsjo added a comment to D67301: [LLD] Use the unified llvm demangle frontend function. NFC..
In D67301#1661748, @rnk wrote:

Should the _ prefix (for itanium symbols in i386 mode) be stripped by the COFF wrapper function, or by the common demangle function (matching _Z or __Z)?

Maybe, I forget if the extra _ comes before the __imp_ prefix or after it when dllimported.

Sat, Sep 7, 12:35 PM · Restricted Project
mstorsjo committed rGc168b4b2a96d: Fix release notes for the MinGW frontend (authored by mstorsjo).
Fix release notes for the MinGW frontend
Sat, Sep 7, 3:57 AM
mstorsjo committed rL371301: Fix release notes for the MinGW frontend.
Fix release notes for the MinGW frontend
Sat, Sep 7, 3:56 AM

Fri, Sep 6

mstorsjo created D67301: [LLD] Use the unified llvm demangle frontend function. NFC..
Fri, Sep 6, 2:02 PM · Restricted Project

Wed, Sep 4

mstorsjo committed rGd581dd501381: [LLD] [COFF] Implement MinGW default manifest handling (authored by mstorsjo).
[LLD] [COFF] Implement MinGW default manifest handling
Wed, Sep 4, 1:38 PM
mstorsjo committed rL370974: [LLD] [COFF] Implement MinGW default manifest handling.
[LLD] [COFF] Implement MinGW default manifest handling
Wed, Sep 4, 1:37 PM
mstorsjo closed D66825: [10/10] [LLD] [COFF] Implement MinGW default manifest handling.
Wed, Sep 4, 1:37 PM · Restricted Project
mstorsjo added inline comments to D66825: [10/10] [LLD] [COFF] Implement MinGW default manifest handling.
Wed, Sep 4, 10:14 AM · Restricted Project