Handling __FILE__ in code size tests in CircleCI

[aheejin]

C++'s __FILE__ macro expands to the file's path. Whether that's a relative path or an absolute one depends on what's given to the compiler. For example,

$ clang++ ../../test.cpp

will expand test.cpp's __FILE__ into ../../test.cpp, whereas

$ clang++ ~/test.cpp

will expand it into /whatever/absolute/path/test.cpp.

In system_lib.py, we use both of them depending on the paths.

• We use relative paths when not using Ninja (build_objects) and batch_inputs is true, which is the default:
  

  emscripten/tools/system_libs.py

  Lines 536 to 539 in f615920

  	elif batch_inputs:
  	# Use relative paths to reduce the length of the command line.
  	# This allows to avoid switching to a response file as often.
  	src = os.path.relpath(src, build_dir)

• We use absolute paths when not using Ninja (build_objects) and batch_inputs is false (Add switch to disable batching of inputs when building system libs #20929)
• We use absolute paths when using Ninja (create_ninja_file). I don't think we can use relative paths in Ninja files.

In CircleCI, tests that depend on build-libs, which is a part of build-linux, which uses Ninja, will be built with absolute paths when code includes __FILE__. This includes all core, other, and browser tests. Other tests (test-mac-arm64, test-windows, ...) don't use build-libs so they use build_objects and thus will include relative paths.

So, this will produce different builds for different CircleCI tests. deterministic_paths parameter, which uses -ffile-prefix-map in system_lib.py makes all relative paths the same string and all absolute paths the same string, but does not make an absolute path and a relative path the same. So even if we make CircleCI always set deterministic_paths, the problem remains:

emscripten/tools/system_libs.py

Lines 495 to 501 in f615920

	if self.deterministic_paths:
	source_dir = utils.path_from_root()
	if batch_inputs:
	relative_source_dir = os.path.relpath(source_dir, build_dir)
	cflags += [f'-ffile-prefix-map={relative_source_dir}/=']
	cflags += [f'-ffile-prefix-map={source_dir}=/emsdk/emscripten',
	'-fdebug-compilation-dir=/emsdk/emscripten']

This problem was brought into attention because the new LLVM 19's libc++abi adds more usage of __FILE__ and one of them ended up in other.test_minimal_runtime_code_size_hello_embind, causing the code sizes to be different between different CircleCI tests. We can make this usage an empty string in release mode (which Zig did) but this does not fundamentally fix the problem that __FILE__ can end up in other code size tests. Currently we seem to use __FILE__ in several places in libraries:

aheejin@aheejin:~/emscripten/system/lib$ grep __FILE__ * -R
compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:      __sanitizer::CheckFailed(__FILE__, __LINE__, \
compiler-rt/lib/builtins/int_util.h:#define compilerrt_abort() __compilerrt_abort_impl(__FILE__, __LINE__, __func__)
libc/musl/include/assert.h:#define assert(x) ((void)((x) || (__assert_fail(#x, __FILE__, __LINE__, __func__),0)))
libcxx/include/__assert:       : _LIBCPP_ASSERTION_HANDLER(__FILE__ ":" _LIBCPP_TOSTRING(__LINE__) ": assertion " _LIBCPP_TOSTRING(            \
libcxx/src/verbose_abort.cpp:  __assert2(__FILE__, __LINE__, __func__, buffer);
libcxxabi/src/abort_message.h:        ::abort_message("%s:%d: %s", __FILE__, __LINE__, __msg);                                                       \
libcxxabi/src/abort_message.cpp:    __assert2(__FILE__, __LINE__, __func__, buffer);
mimalloc/include/mimalloc/types.h:#define mi_assert(expr)     ((expr) ? (void)0 : _mi_assert_fail(#expr,__FILE__,__LINE__,__func__))
wasmfs/support.h:  wasmfs::handle_unreachable(msg, __FILE__, __LINE__)

---

Not sure what is the best way to proceed:

1. Make __FILE__ an empty string in release mode via adding -D__FILE__="" in system_lib.py.
2. Make __FILE__ an empty string when the environment variable CIRCLECI is set, which is set in all CIrcleCI tests. But then we have to make sure to clear cache and set CIRCLECI when rebaselining code size tests on our local machine, which is a pain, like

     $ ./emcc --clear-cache
     $ CIRCLECI=1 ./tools/maint/rebaseline_tests.py

3. ???

I personally prefer 1, which can be as simple as #23196.

[aheejin]

cc @dschuff @sbc100

[sbc100]

I think the better approach is to try to make deterministic_paths actually deterministic regardless of weather we use batch builds.

Alternatively, we should just have CI never use batch building?

[sbc100]

Actually its not just CI.. anyone who want to run this test needs to have the same contents on libc++abi.a.

Another approach then would be to somehow patch out or remove the usage of __FILE__ in libc++abi?

[sbc100]

I found some good info on this problem here: https://reproducible-builds.org/docs/build-path/

I sounds like -fmacro-prefix-map might be the answer?

[sbc100]

It batch builds are really the problem here I'm kind of tempted to remove them, or make them opt-in only, since they cause me other headaches from time to time myself.

[dschuff]

If we can switch batch builds to use absolute paths like the non-batch and Ninja builds, then we could use absolute paths everywhere, and the appropriate command-line flags to make those paths deterministic (or not, for local development debugging). The comment for why it uses relative paths is to avoid response files, but given that we need response file support anyway (because the command-line lengths might be too long anyway) what is the real advantage of that?

[sbc100]

Sure at the very least we could use absolute paths when deterministic_paths is set?

I actually prefer absolute paths because one of the annoying things about the batch builds is that filenames in the error message it produces are not open-able (because they are relative to some build directory, not my directory where I ran the command).

[dschuff]

Actually I think if we just make the new value (the strings on the RHS of the '=') of line 499 and 500 of

emscripten/tools/system_libs.py

Lines 495 to 501 in f615920

	if self.deterministic_paths:
	source_dir = utils.path_from_root()
	if batch_inputs:
	relative_source_dir = os.path.relpath(source_dir, build_dir)
	cflags += [f'-ffile-prefix-map={relative_source_dir}/=']
	cflags += [f'-ffile-prefix-map={source_dir}=/emsdk/emscripten',
	'-fdebug-compilation-dir=/emsdk/emscripten']

match, then the relative and absolute paths should end up matching in deterministic mode, and non-deterministic mode should be unchanged.

[sbc100]

That would be a great solution!

One downside is that the codesize tests will fail without deterministic_paths set... I'm not sure how to fix that.

[sbc100]

How about we use -fmacro-prefix-map in all cases and -ffile-prefix-map/-fdebug-compilation-dir only for deterministic_paths?

[aheejin]

I think the better approach is to try to make deterministic_paths actually deterministic regardless of weather we use batch builds.
Alternatively, we should just have CI never use batch building?

Some of the CI uses batch building while others use Ninja, so removing batch building from CI does not fix the problem. The ones that use Ninja are the ones dependent on build-libs (and build-linux):

emscripten/.circleci/config.yml

Lines 980 to 1021 in 56ee9dd

	- test-sanity:
	requires:
	- build-linux
	- test-posixtest:
	requires:
	- build-linux
	- test-core0:
	requires:
	- build-linux
	- test-core2:
	requires:
	- build-linux
	- test-core3:
	requires:
	- build-linux
	- test-wasm64:
	requires:
	- build-linux
	- test-wasm64-4gb:
	requires:
	- build-linux
	- test-wasm2js1:
	requires:
	- build-linux
	- test-other:
	requires:
	- build-linux
	- test-browser-chrome:
	requires:
	- build-linux
	- test-browser-chrome-2gb:
	requires:
	- build-linux
	- test-browser-chrome-wasm64:
	requires:
	- build-linux
	- test-browser-chrome-wasm64-4gb:
	requires:
	- build-linux
	- test-browser-firefox:
	requires:
	- build-linux

Actually its not just CI.. anyone who want to run this test needs to have the same contents on libc++abi.a.
Another approach then would be to somehow patch out or remove the usage of __FILE__ in libc++abi?

That's basically what Zig guys did (at least for the release mode): ziglang/zig@98a30ac
This can be a quick solution, which I can do, but we still have some uses of __FILE__ in other parts of libraries. They just don't happen to affect the current code size tests.

I found some good info on this problem here: https://reproducible-builds.org/docs/build-path/
I sounds like -fmacro-prefix-map might be the answer?

I think we are already using -ffile-prefix-map which implies -fmacro-prefix-map? Even if we are using them we are using it separately for relative paths and absolute paths, so it makes all different relative paths into a single path and all different absolute paths into a single one but doesn't make an absolute path and a relative path into a single path. Maybe we should consider doing that.

It batch builds are really the problem here I'm kind of tempted to remove them, or make them opt-in only, since they cause me other headaches from time to time myself.

We only need to fix the relative/absolute path thing so I guess we don't need to remove the batch build itself unless it has other problems? But I'm not very familiar with that. It looks the batch build started using relative paths in order not to switch to response file too often.

If we can switch batch builds to use absolute paths like the non-batch and Ninja builds, then we could use absolute paths everywhere, and the appropriate command-line flags to make those paths deterministic (or not, for local development debugging). The comment for why it uses relative paths is to avoid response files, but given that we need response file support anyway (because the command-line lengths might be too long anyway) what is the real advantage of that?

Sure at the very least we could use absolute paths when deterministic_paths is set?

I actually prefer absolute paths because one of the annoying things about the batch builds is that filenames in the error message it produces are not open-able (because they are relative to some build directory, not my directory where I ran the command).

If we use absolute paths everywhere that's not gonna be deterministic because they are different in different CI bots (e.g. test-other and test-mac-arm64) and also different from our local build, no?

Actually I think if we just make the new value (the strings on the RHS of the '=') of line 499 and 500 of

emscripten/tools/system_libs.py

Lines 495 to 501 in f615920

	if self.deterministic_paths:
	source_dir = utils.path_from_root()
	if batch_inputs:
	relative_source_dir = os.path.relpath(source_dir, build_dir)
	cflags += [f'-ffile-prefix-map={relative_source_dir}/=']
	cflags += [f'-ffile-prefix-map={source_dir}=/emsdk/emscripten',
	'-fdebug-compilation-dir=/emsdk/emscripten']

match, then the relative and absolute paths should end up matching in deterministic mode, and non-deterministic mode should be unchanged.

This can be good solution, but this will still generate an "not openable" file paths @sbc100 mentioned. Would that be OK? Also you need to clear cache and rebuild with the deterministic mode every time rebaselining code size tests.

[dschuff]

-ffile-prefix-map subsumes both -fmacro-prefix-map and -fdebug-prefix-map. So I think it's what we want. I think the only problem is that in deterministic mode, the paths that started out relative differ from the paths that started out absolute. So making the RHS of those match as I mentioned would fix that.

[sbc100]

but we still have some uses of __FILE__ in other parts of libraries

BTW, some of these are never used. For example I just tried to make a test case using compilerrt_abort which looks like it uses __FILE__:

int_util.h:#define compilerrt_abort() __compilerrt_abort_impl(__FILE__, __LINE__, __func__)

But actually it ignores all its arguments in most configurations (including ours).