fix Issue 21082 - Testsuite fails on OSX (runnable/test16096.sh with …

[WalterBright]

…asserts on)

This doesn't actually fix it, just trying to find the bug.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	21082	major	Testsuite fails on OSX (runnable/test16096.sh with asserts on)

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#11510"

[WalterBright]

@jacob-carlborg since you develop on OSX a lot, could you please have a go at seeing where the bug is? Thanks!

[jacob-carlborg]

since you develop on OSX a lot, could you please have a go at seeing where the bug is? Thanks!

I've done a bit investigation. The reason why it only fails with assertions turned on is that the length of the underlying array (Barray) is longer than the Rarray.

[jacob-carlborg]

I've done some further investigation. The problem is that the array (SegData) that stores the segments is reset (through some additional calls) here:

dmd/src/dmd/mars.d

Line 714 in b387646

obj_write_deferred(library);

The Objective-C glue code did not expect that. It caches the segments returned by Obj.getsegment here:

dmd/src/dmd/objc_glue.d

Lines 357 to 362 in b387646

	return segments[id] = Obj.getsegment(
	seg.sectionName,
	seg.segmentName,
	seg.alignment,
	seg.flags
	);

If I reset the above segments the assertion pass. The problem is, I don't know the best way to do that. The Objective-C segments are cached in the glue layer, while the other segments are cached in the backend here:

dmd/src/dmd/backend/machobj.d

Lines 229 to 258 in b387646

	/**
	* Section index for the __thread_vars/__tls_data section.
	*
	* This section is used for the variable symbol for TLS variables.
	*/
	int seg_tlsseg = UNKNOWN;
	/**
	* Section index for the __thread_bss section.
	*
	* This section is used for the data symbol ($tlv$init) for TLS variables
	* without an initializer.
	*/
	int seg_tlsseg_bss = UNKNOWN;
	/**
	* Section index for the __thread_data section.
	*
	* This section is used for the data symbol ($tlv$init) for TLS variables
	* with an initializer.
	*/
	int seg_tlsseg_data = UNKNOWN;
	int seg_cstring = UNKNOWN; // __cstring section
	int seg_mod_init_func = UNKNOWN; // __mod_init_func section
	int seg_mod_term_func = UNKNOWN; // __mod_term_func section
	int seg_deh_eh = UNKNOWN; // __deh_eh section
	int seg_textcoal_nt = UNKNOWN;
	int seg_tlscoal_nt = UNKNOWN;
	int seg_datacoal_nt = UNKNOWN;

And initialized and reset here:

https://github.com/dlang/dmd/blob/master/src/dmd/backend/machobj.d#L453-L462

I'm open to suggestions how to best solve this.

[WalterBright]

Thank you, @jacob-carlborg ! I'll look into how best to solve this. The reset is done for the other symbols, I just have to remember where I put that code!

[WalterBright]

Ok, what needs to happen is symbol_reset() must be called on those symbols. This is called by reset_symbols() in machobj.d, which resets symbols in public_symbuf and extern_symbuf, and is called from obj_init().

[jacob-carlborg]

Ok, what needs to happen is symbol_reset() must be called on those symbols. This is called by reset_symbols() in machobj.d, which resets symbols in public_symbuf and extern_symbuf, and is called from obj_init().

That much I've figured out. The problem is that the symbols are in the glue layer and not the backend. Should those be moved to the backend? I assume the backend shouldn't call the glue layer?

[WalterBright]

You're right, the back end shouldn't call the glue layer. But no reason the glue layer can't call a function in the back end and pass it an array of those symbols.

[jacob-carlborg]

But no reason the glue layer can't call a function in the back end and pass it an array of those symbols.

How do you envision that? It's the backend that initiates the reset of the symbols, not the glue layer. Or should a call to reset the symbols be added here, before this line:

dmd/src/dmd/mars.d

Line 714 in 5e7ccd9

obj_write_deferred(library);

[WalterBright]

obj_start() in glue.d is the function you're looking for.

[jacob-carlborg]

obj_start() in glue.d is the function you're looking for.

Oh, I thought that was part of the backend, but I see it's part of the glue layer. I'll take a look, thanks.

[jacob-carlborg]

@WalterBright this PR should fix the issue: #11520.

[Geod24]

Closing in favor of #11520 which has auto-merge on.

[WalterBright]

@jacob-carlborg thank you very much for chasing this down and fixing it!