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

use md5 to generate names for string comdats #6546

Merged
merged 1 commit into from Feb 18, 2017

Conversation

WalterBright
Copy link
Member

Because string literals can be arbitrarily long, and we don't need to ever reverse the comdat name to the string.

This PR also uses this technique for all Windows string literals now. Too bad Elf uses a completely different scheme.

@WalterBright WalterBright force-pushed the md5-comdat branch 2 times, most recently from 3076b7b to 5aad0fd Compare February 17, 2017 02:49
@WalterBright
Copy link
Member Author

The win32 version exposed a bug in the Digital Mars implementation of sscanf(). I've uploaded a fixed snn.lib, but the autotester remains blocked until it updates snn.lib too.

@WalterBright
Copy link
Member Author

Note that this change puts all string literals for Win32 and Win64 into readonly sections. Any code that tries to cast string literals to mutable strings and then write to them will now fail (which is what was wrong with sscanf).

String literals for Elf and Mach systems were already in readonly sections, so this would only affect non-portable Windows code.

@rainers
Copy link
Member

rainers commented Feb 18, 2017

I've uploaded a fixed snn.lib, but the autotester remains blocked until it updates snn.lib too.

Where did you upload it to? Wouldn't it make sense to make the binary library available in a repository where everyone can (automatically) pick it up for a local build (including installers).

@WalterBright
Copy link
Member Author

You can pick it up here:
http://ftp.digitalmars.com/snn.lib

@WalterBright
Copy link
Member Author

WalterBright commented Feb 18, 2017

@MartinNowak prepares the releases, so it's his decision as to the best way to package snn.lib.

@@ -5701,6 +5704,24 @@ Symbol *toStringSymbol(const(char)* str, size_t len, size_t sz, bool comdat = fa
OutBuffer buf;
buf.writestring("__");
mangleToBuffer(se, &buf); // recycle how strings are mangled for templates

if (buf.offset >= 32 + 2)
{ // Replace long string with hash of that string
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"no characters after an opening or closing brace"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @wilzbach - we should tolerate this in the compiler for now, but not in druntime or phobos

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"no characters after an opening or closing brace"

Can't we automate that? It's an easy regex. And it shouldn't be too complex to only consider added lines of the patch.

@MartinNowak
Copy link
Member

@MartinNowak prepares the releases, so it's his decision as to the best way to package snn.lib.

We automatically pick-up and update to the latest libC, see https://github.com/dlang/installer/blob/5179af93d9f0fd84d714e8742492434bf16ed2f6/create_dmd_release/build_all.d#L459. Not a good "release" model, but changes to your libC are rare enough that I hardly care, still would be better with checksum and version.

ubyte u1 = u >> 4;
buf.writeByte((u1 < 10) ? u1 + '0' : u1 + 'A' - 10);
u1 = u & 0xF;
buf.writeByte((u1 < 10) ? u1 + '0' : u1 + 'A' - 10);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Time for a writeHexByte?

@MartinNowak
Copy link
Member

sscanf doesn't occur in this patch, why does it need to be fixed? Win tester is currently failing due to dlang/druntime#1763 et.al.

@WalterBright WalterBright merged commit 4f4c7b1 into dlang:master Feb 18, 2017
@WalterBright
Copy link
Member Author

sscanf doesn't occur in this patch, why does it need to be fixed?

Because putting string literals in RO memory exposed the bug in sscanf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants