Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to D v2.081.0 #2752

Merged
merged 38 commits into from
Jul 3, 2018
Merged

Upgrade to D v2.081.0 #2752

merged 38 commits into from
Jul 3, 2018

Conversation

kinke
Copy link
Member

@kinke kinke commented Jun 18, 2018

[Very early stage, Phobos doesn't compile yet, apparently due to NRVO and special ref variable - at least on Win64; no problem for Linux x64 apparently.]

@kinke
Copy link
Member Author

kinke commented Jun 19, 2018

I included upstream's/Rainer's new dmd/root/longdouble.d, although it's not used yet (this accounts for almost 900 new lines). It's a clean (now all in D + C++ header, no more extra asm file etc.) x87 real_t implementation for compile-time reals on MSVC hosts (MSVC only supports double-precision reals) and I'll probably make that work soon, primarily for consistent compile-time reals on x86[_64] hosts and no cross-compilation issues from Windows -> Linux etc.

@kinke
Copy link
Member Author

kinke commented Jun 19, 2018

Semaphore CI (Ubuntu 18.04) is doing extremely well without self-compilation cycle, only runnable/minimal2.d and std.numeric (debug only) failing.

The AST now features special-ref result variables (storage classes:
ref, temp, result) after rewriting out contracts; from dmd/func.d:

/*   out(id1) { statements1... }
 *   out(id2) { statements2... }
 *   ...
 * becomes:
 *   out(__result) { { ref id1 = __result; { statements1... } }
 *                   { ref id2 = __result; { statements2... } } ... }
 */

We are talking about the `id1` and `id2` variables here.
There's an existing assertion that we don't set a special-ref variable's
lvalue (T**) to the sret pointer (T*) which was already triggered when
compiling Phobos without unittests.
@kinke
Copy link
Member Author

kinke commented Jun 20, 2018

Only runnable/minimal2.d remaining for Semaphore and Travis/Linux, and that checks whether classes with only static shared data members and/or static function members don't require object.d (no TypeInfo emission), which seems of little value to me.
lit-tests and unittests on Win64 working fine on my box as well; I haven't checked dmd-testsuite yet.

Wrt. crashing self-compiled LDC, I guess that's due to the numerous extern(C++) changes (vtable?), and apparently something not covered by dmd-testsuite yet. :/

Use more robust LLVM inline assembly, which works around a `mov` issue
in DMD-style inline asm (at least on Win64) - additional dereferencing
when moving a `longdouble_soft` parameter (passed indirectly by value in
a GP register) to RAX instead of just copying the address.
And revise the Win64 ABI return rules in this function.

Required to make dmd/root/longdouble.d (mostly extern(C++)) compile.
Move the unittest block before the module-level `pure:`, as a pure
unittest function cannot access mutable globals.
@kinke
Copy link
Member Author

kinke commented Jun 21, 2018

With {runnable,compilable}/minimal2.d now disabled, all tests on my Win64 box are green now too.

There's a new need to access a class' vtable symbol, see dlang/dmd#8362.

Use it as alias to the actual vtable symbol with different type (dummy:
`i8*`, actual: `[N x i8*]`) and mangled name.

I tried matching the special symbol's mangled name and using an
appropriate static array front-end type for it, but then casting the
symbol address for the assignment leads to issues if the ctor is @safe.
So I decided to handle it in DtoSymbolAddress().

Unfortunately, this seems not to solve the extern(C++) issues exposed by
LDC self-compilation yet.
dlang/dmd#8359 introduces direct call expressions for struct methods
(which cannot be virtual anyway and are so always called directly).

Our previous code relied on the instance being a class. There are
multiple ways to fix this; I went with this one.
@kinke
Copy link
Member Author

kinke commented Jun 23, 2018

Merging stable and fixing the resulting regressions turned out to be almost as much work as merging beta1. ;)

Unfortunately it didn't fix the self-compilation issues. The extern(C++) vtables/vptrs seem to be totally off if debugging on Windows is to be trusted; an indirect call to isTemplateDeclaration() ending up in toStringExp() of an unrelated class IIRC etc.

@dnadlinger
Copy link
Member

Just to confirm, did you see Iain's recent fixes?

@kinke
Copy link
Member Author

kinke commented Jun 23, 2018

Nope, thx for the hint. I only checked stable, those all sadly made it into master.

I neglected C++ header regressions because it's working fine with all other host compilers, just not with itself.

@dnadlinger
Copy link
Member

Ah, good point.

@kinke
Copy link
Member Author

kinke commented Jun 23, 2018

Oh, I see we have an LDC-specific non-final (but extern(D)) dtor for class Dsymbol, with no corresponding C++ declaration, this may be promising.

It's the only dtor for all front-end classes. Make sure it doesn't mess
up the vtable, as dtors of extern(C++) classes now need an explicit
`final` to be non-virtual.

This fixes the self-compilation issues.
The TypeInfo may need an extern(D) wrapper accounting for ABI
differences wrt. extern(C++) dtor implementations.

This fixes `runnable/cppa.d` for 32-bit x86 targets.
@kinke
Copy link
Member Author

kinke commented Jun 23, 2018

A new Windows-only debuginfo test (runnable/testpdb.d, test18984()) fails, checking whether there's a DI local associated with an NRVO/sret variable - we don't declare any DI variable, independent of target.

OSX also fails due to a new, OSX-specific debuginfo test (runnable/debug_info.d), checking whether the executable contains line infos in a specified segment and section of the executable. According to StackOverflow, the debuginfos are stripped by the linker (from the final executable, but present in the object file), and my quick SSH checks seem to confirm this. I guess they are somehow kept for DMD (non-standard segment?) and disabling the test for LDC is probably safe.
Another issue on OSX is runnable/cppa.d failing for 2 out of 4 runs (Invalid UTF-8 sequence for 32-bit release and segfault on 64-bit debug, working for 32-bit debug and 64-bit release...).

The rest should work.

@jacob-carlborg
Copy link
Contributor

jacob-carlborg commented Jun 27, 2018

Thx for the link. So if I understand this correctly, running dsymutil will create an additional .dSYM file containing the Dwarf data of all object files. druntime's rt.backtrace would have to read & parse that file instead of a section of the executable itself. [This seems very similar to the CodeView .pdb files from MS.]

That's correct. But these files are more used to ship debug symbols with a released application to the users than used during development. During development the linker inserts references to the object files, the debugger will read the references, find the object files on disk and finally read the debug info from the object files. That's why the debugger works with a D compiler even though no .dSYM is generated. This is also easily verified by compiling an executable, remove the object files and then debug the program. The debugger will fail to find the debug symbols.

I quickly looked into how to set Mach-O section flags in LLVM but didn't find anything useful. I guess the .dSYM method is cleaner and what Mac debuggers expect, so I'm inclined to disabling the test for now and relying on a future rt.backtrace support for .dSYM files.

I implemented the current approach because not using .dSYM files is one file less to worry about for the user (developer) (the .dSYM file would need to be shipped with the executable, with the current approach distributing a single binary is enough). This also required less changes in druntime because for Linux and FreeBSD it was looking in a section of the current executable. Also, no need to figuring out the .dSYM, which might be difficult. Apple is not always so open. The debugger will read the debug info correctly with this change as well. It seems like the debugger is looking for specific sections while the linker is looking for attributes on the section.

BTW, please ping me in the future for anything macOS related, like this. I might be able to help and explaining, this might have saved you guys some time instead of having to figuring out this again after I already figured out how it works.

@jacob-carlborg
Copy link
Contributor

I quickly looked into how to set Mach-O section flags in LLVM but didn't find anything useful

I think it would be set in a similar way like this [1]. In that example literal_pointers is a section type and no_dead_strip is a section attribute (flag).

[1]

? "__DATA,__objc_selrefs,literal_pointers,no_dead_strip"

@JohanEngelen
Copy link
Member

@jacob-carlborg Could you try and see whether you can make it work for LDC?

We can then add a flag that allows users to switch between the two possibilities. Having to run dsymutil is a little annoying (I ran into it when doing ASan work), and not many people may know about it (clang runs it for the user) so I'm in favor of having the default be the same as DMD. But I think it's good to be able to switch to dsymutil-based workflow.

@jacob-carlborg
Copy link
Contributor

@JohanEngelen any hints to where this need to be changed/set in LDC?

@kinke
Copy link
Member Author

kinke commented Jun 27, 2018

BTW, please ping me in the future for anything macOS related, like this. I might be able to help and explaining

Will do, but you already did help a lot via the very detailed commit message, thanks!

The debugger will read the debug info correctly with this change as well.

I was thinking about other code analyzers too, profilers etc., but I'm not familiar with Mac.

any hints to where this need to be changed/set in LDC?

That's the main issue. While setSection() works for symbols themselves, the debuginfo stuff is generated 'implicitly' by LLVM AFAICT, and I have no idea if it's possible to override those section flags. Embedding linker command-line args works (at least for the MS linker), but I didn't find any useful ld64 args either.

Having to run dsymutil is a little annoying

Manually, of course; I meant doing it like clang and invoking it automatically after linking; but thinking about it, that would defeat the original idea behind these separate debuginfos (faster linking during iterative development).

@jacob-carlborg
Copy link
Contributor

I was thinking about other code analyzers too, profilers etc., but I'm not familiar with Mac.

True. But I'm not sure which tools would read the debug info.

When installing the static lib archived by LDC (1 D object file, 1 C++
one) via CMake, `ranlib` is apparently invoked for the installed copy,
which may corrupt it, e.g., reproduced via CircleCI SSH with Xcode 9.2
(.a file size unchanged after (successful) ranlib run):

/Applications/Xcode-9.2.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/nm:
foo.a truncated or malformed archive (terminator characters in archive
member "c_" not the correct "`\n" values for the archive member header
at offset 784)

We already encountered this or a very similar bug (with LLVM 5 and some
Xcode version IIRC). Copy the file manually for now as hopefully robust
workaround.
@jacob-carlborg
Copy link
Contributor

Could you try and see whether you can make it work for LDC?

I've done some digging in LLVM and this is what I came up with: the section attribute is set in in the DwarfLineSection field in MCObjectFileInfo::initMachOMCObjectFileInfo here [1]. For macOS 64bit there's a subclass X86_64MachoTargetObjectFile that is used, with the following inheritance chain (most derived to the right):

MCObjectFileInfo -> TargetLoweringObjectFile -> TargetLoweringObjectFileMachO -> X86_64MachoTargetObjectFile

A Target contains a TargetMachine. A TargetMachine contains a MCObjectFileInfo. For macOS 64bit there's a X86TargetMachine target machine, with the following inheritance chain (most derived to the right):

TargetMachine -> LLVMTargetMachine -> X86TargetMachine

The DwarfLineSection field is protected, so in theory it would be possible to subclass X86_64MachoTargetObjectFile to override the default value of DwarfLineSection. This would require subclassing X86TargetMachine, which unfortunately is not possible because it's final 😞.

[1] https://github.com/llvm-mirror/llvm/blob/e8508360bc0544c086058911cc8673d98f35aa4c/lib/MC/MCObjectFileInfo.cpp#L234-L236

@kinke
Copy link
Member Author

kinke commented Jun 28, 2018

@jacob-carlborg: Thanks a lot for checking. There is one viable option - we have our own LLVM fork (mainly used for CI and the prebuilt release packages; the -link-internally feature depends on it too for example, or, more precisely, the LLD fork ;)). As 6.0.1 is about to be released soon, I'd have upgraded anyway, so we could include a tiny patch for Mach-O debug sections while at it.

[We could also enable more LLVM targets for that build. Linux distros seem to default to including all targets.]

I.e., druntime and Phobos too, not just ldc-jit-rt.
[Only affecting MULTILIB=OFF builds, as the fat libs are generated and
copied manually anyway).
@kinke kinke changed the title [WIP] Upgrade to D v2.081.0-beta.2 [WIP] Upgrade to D v2.081.0-rc.1 Jun 28, 2018
@jacob-carlborg
Copy link
Contributor

Thanks a lot for checking. There is one viable option - we have our own LLVM fork (mainly used for CI and the prebuilt release packages; the -link-internally feature depends on it too for example, or, more precisely, the LLD fork ;)). As 6.0.1 is about to be released soon, I'd have upgraded anyway, so we could include a tiny patch for Mach-O debug sections while at it.

I was thinking of that as well, not sure how much customization you're willing to make. As far as I can see there are two ways to patch this: either change the attribute directly or remove the final specifier for X86TargetMachine to allow creating a subclass of X86TargetMachine. I think the second option is more flexible and would be able to support a command line switch that Johan suggested.

@jacob-carlborg
Copy link
Contributor

jacob-carlborg commented Jun 29, 2018

A third option could be, not sure if it's possible to do for LDC, is to duplicate the data in __debug_line in a new custom section without the debug attribute. druntime would read the custom section while all other standard tools should not be affected.

@kinke
Copy link
Member Author

kinke commented Jun 29, 2018

or remove the final specifier for X86TargetMachine to allow creating a subclass of X86TargetMachine

Making the flags configurable at runtime would be nice, but is surely more complicated (we need to remain fully compatible with vanilla LLVM). Edit: well maybe not; couldn't we simply add a Boolean global to MCObjectFileInfo.cpp controlling the Mach-O debug section flags and advertise that feature of our fork via some #define in an LLVM header?

duplicate the data in __debug_line in a new custom section without the debug attribute

I guess this would be doable via some late extra LLVM pass, but having the (potentially large) debug data twice in the object files seems undesirable.

@jacob-carlborg
Copy link
Contributor

we need to remain fully compatible with vanilla LLVM

I thought you weren't, that's why you're using a fork?

[macOS] CMake: Work around ranlib potentially corrupting static libs
@kinke
Copy link
Member Author

kinke commented Jun 29, 2018

Nope, the fork is just used for more advanced features not available (or not properly working) with vanilla LLVM (-link-internally, which requires LLD and a one-liner patch for LLD I haven't gotten round to submitting yet, and emulated TLS for Android, which I know nothing about). Besides LLD, it also contains compiler-rt (e.g., sanitizer and profiling libs) directly in its source tree and prebuilt packages (=> CI).
Semaphore and Travis (except for 1 job) use vanilla LLVM.

@kinke kinke changed the title [WIP] Upgrade to D v2.081.0-rc.1 Upgrade to D v2.081.0-rc.1 Jun 29, 2018
@kinke
Copy link
Member Author

kinke commented Jun 29, 2018

Oh I just realized LLVM 6.0.1 has already been tagged (annoyingly, no tags in git mirrors...). I merged it into our fork and included a Mach-O section hack: ldc-developers/llvm@7798278

@kinke
Copy link
Member Author

kinke commented Jun 29, 2018

I also found the culprit for sporadic std.experimental.allocator unittest failures on Win64 (and OSX too IIRC, not sure about Linux): dlang/phobos#6622

@kinke
Copy link
Member Author

kinke commented Jul 3, 2018

Oh, that stable made it to the final 2.081.0 upstream shortly afterwards.

@kinke kinke changed the title Upgrade to D v2.081.0-rc.1 Upgrade to D v2.081.0 Jul 3, 2018
@kinke kinke merged commit 69104bd into master Jul 3, 2018
@kinke kinke deleted the merge-2.081 branch July 3, 2018 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants