Page MenuHomePhabricator

mstorsjo (Martin Storsjö)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 25 2016, 12:54 PM (181 w, 4 d)

Recent Activity

Tue, Jan 14

mstorsjo committed rG337e4359645e: [libcxx] [Windows] Make a more proper implementation of strftime_l for mingw… (authored by mstorsjo).
[libcxx] [Windows] Make a more proper implementation of strftime_l for mingw…
Tue, Jan 14, 12:32 PM
mstorsjo closed D69554: [libcxx] [Windows] Make a more proper implementation of strftime_l for mingw with msvcrt.dll.
Tue, Jan 14, 12:30 PM · Restricted Project

Mon, Jan 13

mstorsjo added a comment to D69554: [libcxx] [Windows] Make a more proper implementation of strftime_l for mingw with msvcrt.dll.

I'll go ahead and land this one tomorrow, before the branch; it's harmless for other configurations and makes the code more consistent. As the locale support in that crt (msvcrt.dll) is so lacking, I don't see a good way to make a meaningful testcase - the main takeaway of the change is consistency.

Mon, Jan 13, 2:41 PM · Restricted Project
mstorsjo committed rG810b28edb3f6: [ItaniumCXXABI] Make tls wrappers properly comdat (authored by mstorsjo).
[ItaniumCXXABI] Make tls wrappers properly comdat
Mon, Jan 13, 1:43 PM
mstorsjo closed D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.
Mon, Jan 13, 1:43 PM · Restricted Project
mstorsjo added a comment to D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.

@rnk and/or @rsmith - can either of you have a look at the new version of this?

Mon, Jan 13, 12:56 PM · Restricted Project

Sat, Jan 11

mstorsjo added inline comments to D70568: [Support] Possibly use exception handler in the Crash Recovery Context in the same way as global exceptions.
Sat, Jan 11, 11:37 PM · Restricted Project, Restricted Project

Fri, Jan 10

mstorsjo added inline comments to D72399: [Support] Replace Windows __declspec(thread) with thread_local in LLVM_THREAD_LOCAL.
Fri, Jan 10, 4:10 AM · Restricted Project

Thu, Jan 9

mstorsjo accepted D72473: [COFF] Align ARM64 range extension thunks at instruction boundary.

LGTM, but I think we should do the same for RangeExtensionThunkARM as well (with alignment 2).

Thu, Jan 9, 1:04 PM · Restricted Project

Wed, Jan 8

mstorsjo added a comment to D69554: [libcxx] [Windows] Make a more proper implementation of strftime_l for mingw with msvcrt.dll.

Shouldn't there be a test for this new behavior?

Wed, Jan 8, 2:32 PM · Restricted Project
mstorsjo added a comment to D69505: [libcxx] [Windows] Store the lconv struct returned from localeconv in locale_t.

FWIW, I looked into tests, and this seems to fix at least 11 failing tests in libc++ on windows.

Wed, Jan 8, 2:23 PM
mstorsjo added a comment to D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.

@rsmith - does this look right to you?

Wed, Jan 8, 2:23 PM · Restricted Project
mstorsjo committed rG78ce19b7e1dc: [LLD] [COFF] Fix post-commit suggestions for absolute symbol equality (authored by mstorsjo).
[LLD] [COFF] Fix post-commit suggestions for absolute symbol equality
Wed, Jan 8, 12:42 PM
mstorsjo closed D72252: [LLD] [COFF] Fix post-commit suggestions for absolute symbol equality.
Wed, Jan 8, 12:41 PM · Restricted Project

Tue, Jan 7

mstorsjo updated the diff for D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.

Making the tls wrappers comdat on any platform that supports comdat; updated tests accordingly.

Tue, Jan 7, 12:20 PM · Restricted Project
mstorsjo added a comment to D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.

Ping @rsmith

Tue, Jan 7, 10:33 AM · Restricted Project

Mon, Jan 6

mstorsjo created D72252: [LLD] [COFF] Fix post-commit suggestions for absolute symbol equality.
Mon, Jan 6, 3:57 AM · Restricted Project
mstorsjo added inline comments to D71981: [LLD] [COFF] Don't error out on duplicate absolute symbols with the same value.
Mon, Jan 6, 3:57 AM · Restricted Project

Sat, Jan 4

mstorsjo committed rG1737cc750c46: [LLD] [COFF] Don't error out on duplicate absolute symbols with the same value (authored by mstorsjo).
[LLD] [COFF] Don't error out on duplicate absolute symbols with the same value
Sat, Jan 4, 2:38 AM
mstorsjo closed D71981: [LLD] [COFF] Don't error out on duplicate absolute symbols with the same value.
Sat, Jan 4, 2:38 AM · Restricted Project

Fri, Jan 3

mstorsjo added a comment to D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.

Can you add a test case for that crash? Otherwise OK.

as i said. the bug can only occur for one line files. so we can't have a run line and a the crashing line. so i wasn't able to make a test for it.

Fri, Jan 3, 2:20 PM · Restricted Project

Thu, Jan 2

mstorsjo added a comment to D71711: [COFF] Make the autogenerated .weak.<name>.default symbols static.
In D71711#1802223, @rnk wrote:

I guess if the absolute symbol change is enough to unblock you, I'd leave this alone for now. The gnu behavior seems error prone and difficult to implement.

Thu, Jan 2, 9:59 PM · Restricted Project
mstorsjo added inline comments to D71981: [LLD] [COFF] Don't error out on duplicate absolute symbols with the same value.
Thu, Jan 2, 9:57 PM · Restricted Project
mstorsjo added a comment to D72011: [sanitizers][windows] Global/LocalAlloc interception and tests.

Builds fine in my MinGW configuration now, thanks!

Thu, Jan 2, 11:43 AM · Restricted Project, Restricted Project

Tue, Dec 31

mstorsjo added a comment to D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.

This broke my builds.

Tue, Dec 31, 11:55 AM · Restricted Project
mstorsjo committed rGb65ca8e5db6f: Revert "[Diagnostic] Add ftabstop to -Wmisleading-indentation" (authored by mstorsjo).
Revert "[Diagnostic] Add ftabstop to -Wmisleading-indentation"
Tue, Dec 31, 11:46 AM
mstorsjo added a reverting change for rGb47b35ff51b3: [Diagnostic] Add ftabstop to -Wmisleading-indentation: rGb65ca8e5db6f: Revert "[Diagnostic] Add ftabstop to -Wmisleading-indentation".
Tue, Dec 31, 11:46 AM
mstorsjo added a comment to D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation.

This broke my builds. A configure test that tries to compile int main (void) { for( int i = 0; i < 9; i++ ); return 0; } (a single line source file), with -Wall enabled, now triggers failed asserts:

Tue, Dec 31, 2:33 AM · Restricted Project

Mon, Dec 30

mstorsjo added inline comments to D72011: [sanitizers][windows] Global/LocalAlloc interception and tests.
Mon, Dec 30, 12:45 PM · Restricted Project, Restricted Project

Sun, Dec 29

mstorsjo added a comment to D71711: [COFF] Make the autogenerated .weak.<name>.default symbols static.

Are there any other options then, than trying to do what GCC/binutils does, by changing the suffix .default into something that more hopefully is unique? They use the name of another defined (probably non-comdat) external symbol from the same object (that should be unique) as suffix. But that feels like a much larger mess to implement.

FWIW, right now this is blocking me from llvm with clang itself for mingw targets, since we started using an external thread_local variable.

Sun, Dec 29, 3:07 PM · Restricted Project
mstorsjo created D71981: [LLD] [COFF] Don't error out on duplicate absolute symbols with the same value.
Sun, Dec 29, 2:57 PM · Restricted Project

Sat, Dec 28

mstorsjo requested review of D71711: [COFF] Make the autogenerated .weak.<name>.default symbols static.
Sat, Dec 28, 3:08 PM · Restricted Project
mstorsjo reopened D71711: [COFF] Make the autogenerated .weak.<name>.default symbols static.
Sat, Dec 28, 3:08 PM · Restricted Project
mstorsjo added a comment to D71711: [COFF] Make the autogenerated .weak.<name>.default symbols static.

Ok, so apparently this doesn't work, as MS link.exe rejects object files, where a IMAGE_SYM_CLASS_WEAK_EXTERNAL symbol points at a symbol which isn't external - so I had to revert it.

Sat, Dec 28, 2:13 PM · Restricted Project
mstorsjo committed rGbc5b7217dcee: Revert "[COFF] Make the autogenerated .weak.<name>.default symbols static" (authored by mstorsjo).
Revert "[COFF] Make the autogenerated .weak.<name>.default symbols static"
Sat, Dec 28, 1:47 PM
mstorsjo added a reverting change for rG7ca86ee6494d: [COFF] Make the autogenerated .weak.<name>.default symbols static: rGbc5b7217dcee: Revert "[COFF] Make the autogenerated .weak.<name>.default symbols static".
Sat, Dec 28, 1:47 PM
mstorsjo committed rG7ca86ee6494d: [COFF] Make the autogenerated .weak.<name>.default symbols static (authored by mstorsjo).
[COFF] Make the autogenerated .weak.<name>.default symbols static
Sat, Dec 28, 1:17 PM
mstorsjo closed D71711: [COFF] Make the autogenerated .weak.<name>.default symbols static.
Sat, Dec 28, 1:17 PM · Restricted Project
mstorsjo updated the diff for D71711: [COFF] Make the autogenerated .weak.<name>.default symbols static.

Did as suggested. As some of the moved blocks are larger than what was left unmoved, parts of the diff is a bit nonobvious though.

Sat, Dec 28, 4:51 AM · Restricted Project

Fri, Dec 27

mstorsjo added a comment to D71711: [COFF] Make the autogenerated .weak.<name>.default symbols static.

Ping @rnk

Fri, Dec 27, 11:02 AM · Restricted Project

Mon, Dec 23

mstorsjo committed rG5a751e747dbf: [AArch64] [Windows] Use COFF stubs for calls to extern_weak functions (authored by mstorsjo).
[AArch64] [Windows] Use COFF stubs for calls to extern_weak functions
Mon, Dec 23, 2:15 AM
mstorsjo committed rGb774aa1011a0: [ARM] [Windows] Use COFF stubs for calls to extern_weak functions (authored by mstorsjo).
[ARM] [Windows] Use COFF stubs for calls to extern_weak functions
Mon, Dec 23, 2:14 AM
mstorsjo closed D71721: [AArch64] [Windows] Use COFF stubs for calls to extern_weak functions.
Mon, Dec 23, 2:14 AM · Restricted Project
mstorsjo closed D71720: [ARM] [Windows] Use COFF stubs for calls to extern_weak functions.
Mon, Dec 23, 2:14 AM · Restricted Project
mstorsjo committed rG86c9831bb40d: [ItaniumCXXABI] Don't mark an extern_weak init function as dso_local on windows (authored by mstorsjo).
[ItaniumCXXABI] Don't mark an extern_weak init function as dso_local on windows
Mon, Dec 23, 2:14 AM
mstorsjo closed D71716: [ItaniumCXXABI] Don't mark an extern_weak init function as dso_local on windows.
Mon, Dec 23, 2:14 AM · Restricted Project

Sun, Dec 22

mstorsjo added inline comments to D71235: [lldb/Lua] Generate Lua Bindings and Make Them Available to the Script Interpreter.
Sun, Dec 22, 12:15 AM · Restricted Project
mstorsjo committed rG9a3fab97468f: [LLDB] Fix building without SWIG (authored by mstorsjo).
[LLDB] Fix building without SWIG
Sun, Dec 22, 12:06 AM

Fri, Dec 20

mstorsjo added a comment to D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM.

And irrespectively if the ArchSpec vs Architecture design, can you (either of you) comment on the updated form of the patch?

The code still seems somewhat schizophrenic to me. :/ The line tables are fixed up super late,

Fri, Dec 20, 5:02 AM · Restricted Project

Thu, Dec 19

mstorsjo created D71721: [AArch64] [Windows] Use COFF stubs for calls to extern_weak functions.
Thu, Dec 19, 11:22 AM · Restricted Project
mstorsjo created D71720: [ARM] [Windows] Use COFF stubs for calls to extern_weak functions.
Thu, Dec 19, 11:13 AM · Restricted Project
mstorsjo created D71716: [ItaniumCXXABI] Don't mark an extern_weak init function as dso_local on windows.
Thu, Dec 19, 9:43 AM · Restricted Project
mstorsjo created D71711: [COFF] Make the autogenerated .weak.<name>.default symbols static.
Thu, Dec 19, 8:54 AM · Restricted Project
mstorsjo added a comment to D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.
In D71572#1788786, @rnk wrote:

Looks like @rsmith did this here:
https://github.com/llvm/llvm-project/commit/fbe2369f1a514423e4c25417ab3532502fde6f2a

I see that it was replacing a CHECK for a specific comdat group with no comdat at all. I *think* that's not correct, we should be checking for the trivial comdat group, the one for the wrapper function, not the comdat group of the TLS variable. Let's get input from Richard, though.

Thu, Dec 19, 8:31 AM · Restricted Project
mstorsjo added a comment to D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM.

@clayborg - can you follow up on @labath's reply here?

Thu, Dec 19, 8:21 AM · Restricted Project

Dec 19 2019

mstorsjo committed rGf20fc65887e2: [clang] Fix compilation with GCC < 8 for MinGW (authored by mstorsjo).
[clang] Fix compilation with GCC < 8 for MinGW
Dec 19 2019, 2:16 AM
mstorsjo closed D71650: [AST] Fix compilation with GCC < 8 for MinGW.
Dec 19 2019, 2:16 AM · Restricted Project
mstorsjo committed rG29d8c27c6528: [LLD] [COFF] Fix reporting duplicate errors for absolute symbols (authored by mstorsjo).
[LLD] [COFF] Fix reporting duplicate errors for absolute symbols
Dec 19 2019, 2:16 AM
mstorsjo closed D71679: [LLD] [COFF] Fix reporting duplicate errors for absolute symbols.
Dec 19 2019, 2:15 AM · Restricted Project

Dec 18 2019

mstorsjo created D71679: [LLD] [COFF] Fix reporting duplicate errors for absolute symbols.
Dec 18 2019, 2:10 PM · Restricted Project
mstorsjo added a comment to D71650: [AST] Fix compilation with GCC < 8 for MinGW.

Ok, so if you're in agreement on this one, can either of you give the stamp of approval? :-)

Dec 18 2019, 1:41 PM · Restricted Project
mstorsjo added a comment to D71650: [AST] Fix compilation with GCC < 8 for MinGW.

Wow, that's novel. Please add a comment explaining that this is a compiler workaround, but otherwise LGTM.

Dec 18 2019, 10:56 AM · Restricted Project
mstorsjo created D71650: [AST] Fix compilation with GCC < 8 for MinGW.
Dec 18 2019, 1:04 AM · Restricted Project

Dec 17 2019

mstorsjo added inline comments to D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.
Dec 17 2019, 2:37 PM · Restricted Project
mstorsjo added inline comments to D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.
Dec 17 2019, 2:26 PM · Restricted Project
mstorsjo updated the diff for D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.

Changed to make it a comdat; unless I'm mistaken, neither linkonce_odr nor weak_odr actually make it possible to link multiple copies of the symbol on windows, unless it's made a comdat. With that in place, the previous change (weak_odr to linkonce_odr) actually isn't needed.

Dec 17 2019, 4:25 AM · Restricted Project
mstorsjo added inline comments to D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.
Dec 17 2019, 4:16 AM · Restricted Project
mstorsjo added inline comments to D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.
Dec 17 2019, 12:47 AM · Restricted Project
mstorsjo committed rGd39510ec1cda: [lit] [windows] Make sure to convert all path separators to backslashes in NT… (authored by mstorsjo).
[lit] [windows] Make sure to convert all path separators to backslashes in NT…
Dec 17 2019, 12:10 AM
mstorsjo committed rGee0a3b5c776c: [MinGW] Implicitly add .exe suffix if not provided (authored by mstorsjo).
[MinGW] Implicitly add .exe suffix if not provided
Dec 17 2019, 12:09 AM
mstorsjo closed D71490: [lit] [windows] Make sure to convert all path separators to backslashes in NT style \\?\... paths.
Dec 17 2019, 12:09 AM · Restricted Project
mstorsjo closed D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided.
Dec 17 2019, 12:09 AM · Restricted Project

Dec 16 2019

mstorsjo added inline comments to D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.
Dec 16 2019, 2:35 PM · Restricted Project
mstorsjo added a comment to D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.

Actually, this doesn't seem to be enough for the case at hand (used with -ffunction-sections -fdata-sections), I'll have to continue looking into it tomorrow.

Dec 16 2019, 2:30 PM · Restricted Project
mstorsjo created D71572: [ItaniumCXXABI] Make tls wrappers properly comdat.
Dec 16 2019, 2:26 PM · Restricted Project
mstorsjo added a comment to D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided.

Ping

Dec 16 2019, 2:26 PM · Restricted Project

Dec 13 2019

mstorsjo added inline comments to D71490: [lit] [windows] Make sure to convert all path separators to backslashes in NT style \\?\... paths.
Dec 13 2019, 3:27 PM · Restricted Project
mstorsjo added a comment to D71490: [lit] [windows] Make sure to convert all path separators to backslashes in NT style \\?\... paths.

Admittedly, this python doesn't really pass all tests anyway, as some tests rely on path formatting to match between different components - but it does help running some number of tests with it at least.

Dec 13 2019, 1:55 PM · Restricted Project
mstorsjo created D71490: [lit] [windows] Make sure to convert all path separators to backslashes in NT style \\?\... paths.
Dec 13 2019, 1:37 PM · Restricted Project
mstorsjo updated the diff for D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided.

Added a code comment, using llvm::path::has_extension, added testcases.

Dec 13 2019, 1:27 PM · Restricted Project

Dec 12 2019

mstorsjo added a comment to D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM.

So my idea would be:
1 - add all virtual functions to from lldb_private::Architecture as normal members of ArchSpec.
2 - have ArchSpec grap the the lldb_private::Architecture using info in the ArchSpec when it is needed for methods added in step 1 and still call through to the lldb_private::Architecture plug-ins

Dec 12 2019, 10:52 PM · Restricted Project
mstorsjo updated the diff for D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM.

Added ArchSpec::GetOpcodeLoadAddress() (didn't refactor Architecture to use it yet, so this is still duplicated code with the ArchitectureArm and ArchitectureMips plugins, but posting for feedback).

Dec 12 2019, 5:37 AM · Restricted Project
mstorsjo created D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided.
Dec 12 2019, 1:30 AM · Restricted Project

Dec 11 2019

mstorsjo added a comment to D70450: [AArch64] Teach Load/Store optimizier to rename store operands for pairing..

Yeah, there were some cases with non-debug $noreg operands. Fixed by rG2675a3c8806a

Dec 11 2019, 12:18 PM · Restricted Project
mstorsjo added a comment to D70450: [AArch64] Teach Load/Store optimizier to rename store operands for pairing..

We're seeing an assertion error in Clang when compiling compiler-rt builtins for aarch64-linux-gnu with this change:

FAILED: CMakeFiles/clang_rt.builtins-aarch64.dir/multc3.c.o 
/b/s/w/ir/k/recipe_cleanup/clanga0lNQB/llvm_build_dir/./bin/clang --target=aarch64-unknown-linux-gnu --sysroot=/b/s/w/ir/k/cipd/linux-arm64 -DVISIBILITY_HIDDEN  -O2 -g -DNDEBUG    -std=c11 -fPIC -fno-builtin -fvisibility=hidden -fomit-frame-pointer -MD -MT CMakeFiles/clang_rt.builtins-aarch64.dir/multc3.c.o -MF CMakeFiles/clang_rt.builtins-aarch64.dir/multc3.c.o.d -o CMakeFiles/clang_rt.builtins-aarch64.dir/multc3.c.o   -c /b/s/w/ir/k/llvm-project/compiler-rt/lib/builtins/multc3.c
clang-10: /b/s/w/ir/k/llvm-project/llvm/include/llvm/MC/MCRegisterInfo.h:677: llvm::MCRegUnitIterator::MCRegUnitIterator(llvm::MCRegister, const llvm::MCRegisterInfo *): Assertion `Reg && "Null register has no regunits"' failed.
clang-10: error: unable to execute command: Aborted
clang-10: error: clang frontend command failed due to signal (use -v to see invocation)

Looks like the patch missed to skip debug operands in an assertion. I will push a fix shortly.

Should be fixed by rG4fe92abceb9a

Dec 11 2019, 10:49 AM · Restricted Project
mstorsjo added a comment to D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM.
  • I think we ought to have some kind of a utility function to hold the logic for the &~1 stuff. there is something like that in Architecture::GetOpcodeLoadAddress. The Architecture class is mostly independent of a target, so we may be able create one instance for each module or symbol file, but that feels quite heavy. I might even go for putting something like that into the ArchSpec class. The raison d'être of the Architecture class was to evict some heavy code out of ArchSpec -- this was ArchitectureArm::OverrideStopInfo. That one is definitely too heavy, so still don't thing it belongs in ArchSpec, but the &~1 thingy seems like it could find a place there.)
Dec 11 2019, 1:43 AM · Restricted Project
mstorsjo committed rGaf39708c2d48: [llvm-readobj] Fix/improve printing WinEH unwind info for linked PE images (authored by mstorsjo).
[llvm-readobj] Fix/improve printing WinEH unwind info for linked PE images
Dec 11 2019, 12:31 AM
mstorsjo closed D71303: [llvm-readobj] Fix/improve printing WinEH unwind info for linked PE images.
Dec 11 2019, 12:31 AM · Restricted Project

Dec 10 2019

mstorsjo updated the summary of D71303: [llvm-readobj] Fix/improve printing WinEH unwind info for linked PE images.
Dec 10 2019, 2:18 PM · Restricted Project
mstorsjo created D71303: [llvm-readobj] Fix/improve printing WinEH unwind info for linked PE images.
Dec 10 2019, 2:18 PM · Restricted Project
mstorsjo committed rGa0f72441c898: [LLDB] [PECOFF] Make sure to set the address byte size in m_data after parsing… (authored by mstorsjo).
[LLDB] [PECOFF] Make sure to set the address byte size in m_data after parsing…
Dec 10 2019, 4:03 AM
mstorsjo closed D71108: [LLDB] [PECOFF] Make sure to set the address byte size in m_data after parsing headers.
Dec 10 2019, 4:03 AM · Restricted Project
mstorsjo added a comment to D71108: [LLDB] [PECOFF] Make sure to set the address byte size in m_data after parsing headers.

@labath does this one seem sensible and like what you suggested in D70848?

Dec 10 2019, 1:19 AM · Restricted Project

Dec 8 2019

mstorsjo added a comment to D71117: Reland "Enable `-funwind-tables` flag when building libunwind".

@mstorsjo thanks for verifying!

When preparing this patch I've looked through the flags being added and couldn't find those that would benefit from this.

  1. Obviously, this won't affect any -W flags.
  2. Same for -fstrict-aliasing.
  3. -EHsc is MSVC-specific and won't generate any new calls (will it?).
  4. -fno-exceptions and -fno-rtti won't add any new calls either.
  5. Same for -nostdinc++.
  6. -D and -U are only for the preprocessor.

    So, it seems like here we only want -funwind-tables to be guarded with this CMake setting.

but it still might be good for consistency in any case?

Now that you've demonstrated the issues that this flag might uncover, my opinion is we probably only want this flag to be set in places we know for sure it is needed, with a clear explanation why it is needed.

Dec 8 2019, 2:33 PM · Restricted Project
mstorsjo added a comment to D71117: Reland "Enable `-funwind-tables` flag when building libunwind".

Thanks - this one seems to work fine for my use case at least.

Dec 8 2019, 1:48 PM · Restricted Project

Dec 6 2019

mstorsjo added a comment to D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM.

So some background on how address masks are handled in LLDB:

Typically the way we have tried to take care of the extra Thumb bit for ARM using:

lldb::addr_t Address::GetCallableLoadAddress(Target *target, bool is_indirect = false) const;
lldb::addr_t GetOpcodeLoadAddress(Target *target, AddressClass addr_class = AddressClass::eInvalid) const;

The first will add the extra bit to an address if needed. The latter will strip the bit if needed. This does require a target though and the target uses the "Architecture" class for ARM to do the work of using the mask. Not sure if we want to try to get an architecture class and use that here for stripping the bit instead of using an address mask?

So, where do I get hold of an Architecture* object instance from within the relevant contexts (within SymbolFileDWARF, where we have a reference to the object file)? Also within source/Symbol/DWARFCallFrameInfo.cpp where we have existing code that does the same.

Well... that's the tricky part (which I've tried to allude to in D70840#1763639. Currently the only thing holding an architecture object is the target class, but since one of the goals of the SymbolFile stuff is to be independent of any particular target, you cannot easily get hold of that. That is why I tried to steer this towards having this in the ArchSpec class. If we don't want that, we'll probably have to create one new Architecture instance local to each Module object...

Ah, sorry for overlooking those parts of your comment. @clayborg - what do you think of @labath's suggestions here?

Dec 6 2019, 3:34 AM · Restricted Project
mstorsjo created D71108: [LLDB] [PECOFF] Make sure to set the address byte size in m_data after parsing headers.
Dec 6 2019, 3:34 AM · Restricted Project
mstorsjo added inline comments to D70848: [LLDB] Set the right address size on output DataExtractors from ObjectFile.
Dec 6 2019, 3:34 AM · Restricted Project

Dec 5 2019

mstorsjo added a comment to D69950: Reapply "Fix crash on switch conditions of non-integer types in templates".
In D69950#1771515, @rnk wrote:

I fixed this particular code upstream:
https://github.com/KhronosGroup/glslang/pull/2010
I am not enough an expert to be sure, but I suspect this is in the area of "invalid, no diagnostic required", where this code is invalid, but a conforming C++ implementation could either reject or accept it. Now we reject it, and that seems better in the long term, even though it creates a fire drill in the short term. =(

Dec 5 2019, 3:12 PM · Restricted Project
mstorsjo added a comment to D69950: Reapply "Fix crash on switch conditions of non-integer types in templates".

This (when reapplied in https://reviews.llvm.org/rG878a24ee244a24c39d1c57e9af2) broke compilation of code that earlier built fine. A reduced example:

namespace glslang {
class TPoolAllocator {
  void operator=(TPoolAllocator);
};
template <class> class a {
  TPoolAllocator *b;
  void c() { allocator = *b; }
  TPoolAllocator allocator;
};
} // namespace glslang

With this patch, some errors in templates are diagnosed earlier (i.e. does not wait till instantiation). Since 'allocator' and 'b' aren't dependent, I think this is a valid diagnosis. GCC throws an error on this code upon instantiation. https://godbolt.org/z/X9Y-Vy

Dec 5 2019, 11:01 AM · Restricted Project