[SAoC] Implement -mangle-prefix

[ahmetsait]

This PR tries to implement -mangle-prefix=<path>:<prefix>

Current status:

// test.d
module test;

__gshared _MyClass_ _c_ = new _MyClass_;
__gshared _MyStruct_ _s_;

float _pi_ = 3.14f;

int _foo_(int x, _MyClass_ c, _MyStruct_ s)
{
    return x * x;
}

interface _MyInterface_
{
    long _bar_();
}

class _MyClass_ : _MyInterface_
{
    long _bar_()
    {
        return 5;
    }
}

struct _MyStruct_
{
    double d;
}

void main()
{
    (new _MyClass_)._bar_();
}

$ src/build.d && generated/linux/release/64/dmd -c -g -mangle-prefix=test.d:_prefix_ test.d && nm test.o
Success
0000000000000000 t 
                 U _D10TypeInfo_d6__initZ
0000000000000000 V _D11TypeInfo_xd6__initZ
                 U _D14TypeInfo_Class6__vtblZ
                 U _D14TypeInfo_Const6__vtblZ
                 U _D15TypeInfo_Struct6__vtblZ
                 U _D18TypeInfo_Interface6__vtblZ
0000000000000000 V _D36TypeInfo_S8_prefix_4test10_MyStruct_6__initZ
0000000000000000 V _D39TypeInfo_C8_prefix_4test13_MyInterface_6__initZ
                 U _D6Object7__ClassZ
                 U _D6object6Object5opCmpMFCQqZi
                 U _D6object6Object6toHashMFNbNeZm
                 U _D6object6Object8opEqualsMFCQtZb
                 U _D6object6Object8toStringMFZAya
0000000000000000 W _D8_prefix_4test10_MyStruct_11__xopEqualsFKxSQBrQBlQBjKxQmZb
0000000000000000 R _D8_prefix_4test10_MyStruct_6__initZ
0000000000000000 W _D8_prefix_4test10_MyStruct_9__xtoHashFNbNeKxSQBsQBmQBkZm
0000000000000000 D _D8_prefix_4test12__ModuleInfoZ
0000000000000000 V _D8_prefix_4test13_MyInterface_11__InterfaceZ
                 U _D8_prefix_4test13_MyInterface_11__InterfaceZ
0000000000000018 D _D8_prefix_4test3_c_CQtQm9_MyClass_
0000000000000020 D _D8_prefix_4test3_s_SQtQm10_MyStruct_
0000000000000000 D _D8_prefix_4test4_pi_f
0000000000000000 W _D8_prefix_4test5_foo_FiCQxQq9_MyClass_SQBmQBg10_MyStruct_Zi
0000000000000000 W _D8_prefix_4test9_MyClass_5_bar_MFZl
0000000000000000 V _D8_prefix_4test9_MyClass_6__initZ
0000000000000000 V _D8_prefix_4test9_MyClass_6__vtblZ
0000000000000000 V _D8_prefix_4test9_MyClass_7__ClassZ
0000000000000000 W _Dmain
                 U _Dmain
                 U _GLOBAL_OFFSET_TABLE_
0000000000000000 t _THUNK0
                 U __start_minfo
                 U __stop_minfo
                 U _d_dso_registry
                 U _d_newclass
                 U _d_run_main
                 U _main
0000000000000000 d internal
0000000000000000 W main

Constructive criticism is welcome.

[dlang-bot]

Thanks for your pull request and interest in making D better, @ahmetsait! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#13115"

[ibuclaw]

Bear in mind that I've been an outsider to the review process for saoc in general, and not knowing just how much of the implementation idea was written down in the proposal.

What rationale is there for adding yet another command-line switch to the compiler?

[maxhaton]

This is solely intended to be available to things like dub so one can depend on multiple versions of a given module in the same dependency tree. I guess in an ideal world you'd have some other abstraction for the option since it's not really for human consumption but there isn't one (env? Probably worse) available for dmd at least.

On Fri, 1 Oct 2021, 08:14 Iain Buclaw, @.***> wrote:

Bear in mind that I've been an outsider to the review process for saoc in general, and not knowing just how much of the implementation idea was written down in the proposal.

What rationale is there for adding yet another command-line switch to the compiler?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dlang/dmd/pull/13115#issuecomment-931970616, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLI75BTVVMAASQ6YQJEBKLUEVNVLANCNFSM5FCYSHRQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[ibuclaw]

This is solely intended to be available to things like dub so one can depend on multiple versions of a given module in the same dependency tree. I guess in an ideal world you'd have some other abstraction for the option since it's not really for human consumption but there isn't one (env? Probably worse) available for dmd at least.

What about prefix mapping? We already have -mv for read this module/package from this directory. But we don't have give this module/package this prefix which could be useful for reproducible builds (i.e: templates that have __PATH__ or __FULL_FILE_PATH__ encoded in their symbol).

[ahmetsait]

What about prefix mapping? We already have -mv for read this module/package from this directory. But we don't have give this module/package this prefix which could be useful for reproducible builds (i.e: templates that have __PATH__ or __FULL_FILE_PATH__ encoded in their symbol).

Could you elaborate a bit on what you're trying to suggest?

[ibuclaw]

Could you elaborate a bit on what you're trying to suggest?

When compiling modules present in a directory named old, change all references to them as if they came from directory new. i.e:

// dmd -prefix-map=$PWD=/build pkg/mod.d
enum fullPath = __FILE_FULL_PATH__; // = "/build/pkg/mod.d"; not "/path/to/pkg/mod.d"

For example, every minor release of gdc in debian/ubuntu breaks ABI with the previous build of libphobos for the same version number, even when there's been no code changes. Why? Because the builder directory differs, and there's a number of templates that encode __FULL_FILE_PATH__ into the mangled symbol.

[ahmetsait]

@ibuclaw anything else I'm missing? Also how can I reproduce that failing DAutoTest build? Not sure if it has anything to do with me.

[ahmetsait]

I see there are some mangling tests I can look up but they don't seem to test invisible/auto-generated symbols so I need a bit of guidance on how to flesh out coverage tests for this PR. run.d is also not exactly easy to follow either.

[MoonlightSentinel]

I see there are some mangling tests I can look up but they don't seem to test invisible/auto-generated symbols so I need a bit of guidance on how to flesh out coverage tests for this PR.

A dshell test might be best suited if you want to create a test based on nm as mentioned in the PR description.

run.d is also not exactly easy to follow either.

See the README.md's in test and it's subdirectories.

[ahmetsait]

I consider this PR ready for review.

[maxhaton]

A runnable test would be helpful - just to show that all the druntime stuff is still working.

[PetarKirov]

Ideally, there should be a repository containing program(s) linking multiple versions of the same library(s). The program should do things like:

• Call functions, read-write global (TLS and shared) variables, check the side-effects of library module constructors. For example:
  • [email protected]:

  module foo.bar;
  
  int global;
  static this() { x = 21; }
  int get() { return global; }
  

  • [email protected]:

  module foo.bar;
  
  int global;
  static this() { x = 42; }
  int get() { return global; }
  

  • main.d:

  module main;
  
  // -mangle-prefix=libfoo-1.2.3/libfoo/src/foo/*:[email protected]
  import [email protected] : get123 = get;
  
  // -mangle-prefix=libfoo-2.1.3/libfoo/src/foo/*:[email protected]
  import [email protected] : get213 = get;
  
  void main()
  {
      assert(get123() == 21);
      assert(get213() == 42);
  }
  

• Should iterate ModuleInfo
• Should use Object.factory on different classes having the same fully qualified name
• Should catch nested exceptions (exception classes nested within functions) from different libraries (this relies on exceptions' typeinfos being unique)
• Should link libraries both statically and dynamically
• etc.

[maxhaton]

There will be.

On Thu, 14 Oct 2021, 23:08 Petar Kirov, @.***> wrote:

Ideally, there should be a repository containing program(s) linking multiple versions of the same library(s). The program should do things like:

• Call functions, read-write global (TLS and shared) variables, check the side-effects of library module constructors. For example:
  • @.***:

module foo.bar; int global;static this() { x = 21; }int get() { return global; }

• @.***:

module foo.bar; int global;static this() { x = 42; }int get() { return global; }

• main.d:

module main; // @.*** @.*** : get123 = get; // @.*** @.*** : get213 = get; void main() { assert(get123() == 21); assert(get213() == 42); }

• Should iterate ModuleInfo
• Should use Object.factory on different classes having the same fully qualified name
• Should catch nested exceptions (exception classes nested within functions) from different libraries (this relies on exceptions' typeinfos being unique)
• Should link libraries both statically and dynamically
• etc.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/dlang/dmd/pull/13115#issuecomment-943773743, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLI75DKOCRHTDBMBJKLUUTUG5IEJANCNFSM5FCYSHRQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[nordlow]

What's the use case for prefixing mangled symbols?

[ahmetsait]

module main;

// -mangle-prefix=libfoo-1.2.3/libfoo/src/foo/*:[email protected]
import [email protected] : get123 = get;

// -mangle-prefix=libfoo-2.1.3/libfoo/src/foo/*:[email protected]
import [email protected] : get213 = get;

void main()
{
    assert(get123() == 21);
    assert(get213() == 42);
}

@PetarKirov FYI using package versions explicitly in code is outside the scope of this project, but I understand it could be an interesting area to explore. Regardless, I will be writing more general tests once I have -import-local implementation sorted out.

What's the use case for prefixing mangled symbols?

@nordlow it eliminates name collision while linking in different versions of the same symbol. This in turn makes using multiple versions of the same package possible with few caveats. (See: https://stephencoakley.com/2019/04/24/how-rust-solved-dependency-hell)

[maxhaton]

Needs rebase.

@RazvanN7 can we look towards merging this?

[RazvanN7]

@maxhaton I don't feel comfortable merging this without @WalterBright 's or @atilaneves approval. I will make a pass on this, however, I am by no means knowledgeable on how this is supposed to work in conjunction with dub

[maxhaton]

What about internal or exported symbols in a library with extern(C) or other non-D mangling?

I don't think this specific case should be supported. If the feature definitely works for normal D code then we can see about custom mangling schemes and such, but for now I think this feature (when used inside dub) should be considered more like something to keep the lights on when the dependencies get weird rather than a way to have everything automagically work (The latter is probably possible in principle but is looking to be beyond the scope of this project, i.e. the work required to get (say) a custom mangle-string on a symbol playing nicely with a different version seems too much for the general case at least)

[rainers]

What about internal or exported symbols in a library with extern(C) or other non-D mangling?

I don't think this specific case should be supported.

I agree, it doesn't have to be supported, but the limitations should be spelled out somewhere (some are also given in the rust-praise in the article linked in the overview, including the interface issue described above).

I wonder if changing the package name rather than only the mangling has been considered: It will result in runtime type information that can be distinguished, but might cause issues with compile time introspection. I suspect type mismatches would be reported more clearly by the compiler, while it will only show a confusing "T != T" if the difference is hidden in the mangling.

[WalterBright]

This sure sounds like the existing -mv switch.

https://dlang.org/dmd-windows.html#switch-mv

[maxhaton]

I'm not sure it is. The idea here is to allow a way for dub to let 2 versions to coexist. Isn't -mv= for forcing (say) "2" versions to actually point to one?

This doesn't actually fly in situations where you don't own the code because the dependencies could be using (say) a feature with different behaviour from each version so picking any one of the 2 would break a dependency.

On Mon, 1 Nov 2021, 09:22 Walter Bright, @.***> wrote:

This sure sounds like the existing -mv switch.

https://dlang.org/dmd-windows.html#switch-mv

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/dlang/dmd/pull/13115#issuecomment-956068729, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLI75EWWKXD7VC6OVHP3ALUJZL5DANCNFSM5FCYSHRQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[dkorpel]

Is this still being pursued?

[ahmetsait]

Despite that I want to push this forward, my current focus is on my mental well-being and university studies. It is rather hard to find time and energy when every class I take requires work-heavy term projects. I would love to come back to this, but I'm afraid it is not a priority right now.

[ahmetsait]

Rebased. Anything else that needs to be done or design discussions to be made? (Not sure about the CI, seems like a random failure to me.)

[thewilsonator]

That indeed is on old error, no idea why that is still occurring, considering you just rebased it.