It is not possible to get .asm files handled as assembly with C preprocessor files

[burkpojken]

Feature requests:
In our big projects we use our own tool chain.
Unfortunately all our hand written assembly files are named .asm instead of .S
Bazel handles .asm as .s files (lower case s) and then they are not seen as preprocessed and no .d file is requested.
Our .asm files can use #include and then the target is not rebuilt when a dependent .h file is changed.
I is a big thing for us to rename all .asm files since there are very many files in many different projects and repos. There are also tools that depend on the .asm extension.

We have patched bazel to handle the .asm files as .S files to make it work now but we would rather have the support in the official bazel releases.

We have tried with the following added to our CROSSTOOL file but the feature is ignored by bazel for the assembly files
feature {
name: 'dependency_file'
flag_set {
action: 'assemble'
action: 'preprocess-assemble'
action: 'c-compile'
action: 'c++-compile'
action: 'c++-module-compile'
action: 'objc-compile'
action: 'objc++-compile'
action: 'c++-header-preprocessing'
action: 'c++-header-parsing'
action: 'clif-match'
expand_if_all_available: 'dependency_file'
flag_group {
flag: '-MD'
flag: '-MF'
flag: '%{dependency_file}'
}
}
}

We use:
Host operating system: Linux
Bazel version: 0.10.1 with patches for the above

[hlopko]

cc @meteorcloudy
Yun is going to work on configuring extensions from crosstool, Yun, will your work cover this use case?

[meteorcloudy]

Yes, I believe this will eventually be solved by making the extensions configurable on different platforms.

[burkpojken]

Sounds good, is there time plan for it?

[meteorcloudy]

@burkpojken I'm planning to make it happen next quarter

[burkpojken]

Ok thanks for info, then we will use our patched version until then

[meteorcloudy]

@burkpojken Which patch did you apply to Bazel to make this work?
I'm asking because I just realized Bazel does recognize .asm file so I don't understand what's the exact issue here.

[burkpojken]

The problem is that .asm files are not handled as preprocess-assemble, so if the .asm files use include it will not be rebuilt even if a header file is changed.
I change so that .asm files are handled as preprocess-assemble instead of just "assemble"
It would be good if this was possible to configure it via CROSSTOOL "dependency_file"

[burkpojken]

To be clearer I changed in CppFileTypes.java

public static final FileType ASSEMBLER_WITH_C_PREPROCESSOR =
new FileType() {
final String ext = ".S";
@OverRide
public boolean apply(String path) {
return path.endsWith(ext) || path.endsWith(".asm");
}
@OverRide
public List getExtensions() {
return ImmutableList.of(ext, ".asm");
}
};

And removed the ".asm" part in
public static final FileType ASSEMBLER

[meteorcloudy]

@burkpojken Thanks!

[burkpojken]

Any progress on this?

[meteorcloudy]

@mhlopko Is it going to be easier to configure extensions of input files after we migrate cc rules to Skylark? Because currently we can only configure extensions of output files of Bazel in CROSSTOOL, which doesn't cover this use case.

[hlopko]

Nope, that won't help, I was hoping your work on configuring extensions for windows will cover it. So it stays as a separate feature request, with a genrule doing the renaming as a workaround.

[emusand]

I work in the same group as burkpojken. I can try to implement this in bazel, but I need some help to find a proper solution.

As burkpojken said, the problem is that the following assembly file extensions are hard-coded in bazel:

• ".S" is assumed to be hand-written assembler, and is preprocessed by the C preprocessor.
• ".s" and ".asm" are assumed to be assembler code generated by a compiler, and are not preprocessed.

All our hand-written assembly files use file extension ".asm". They need to be preprocessed. It is not feasible for us to rename all these files from ".asm" to ".S". (mhlopko's workaround, to use a genrule to rename the files, would work but is a bit ugly.)

The simplest solution is to use our current patch, where we just hard-code that ".asm" files should be preprocessed. But can we push such a patch upstreams? Then all ".asm" files will be preprocessed, unnecessarily for other users as their ".asm" files do not contain any preprocessor directives.

A better solution is probably to make it possible to set which file extensions to preprocess in the CROSSTOOL file. Do you have any proposal of the syntax of this new setting?

[hlopko]

This breaks quite a bit of Google, so this change as is would have to go through the incompatible change process.

But we do have a way to specify extensions in the CROSSTOOL (artifact name patterns). But apparently we don't use them for assembly :/

[emusand]

Aha. I am a bit surprised to hear that it broke internal tests at Google. The output from the preprocessor should be the same as the input, if the .asm files do not contain any preprocesor directives, so I thought that the only difference would be a slight performance hit.

Anyway, I agree that making this configurable in CROSSTOOL is a good idea. Do you have any suggestion on how to use the artifact name patterns message to configure this? I can try to implement it in Bazel.

[burkpojken]

I think it is not really consequent how this is implemented in Bazel today.
The default for the feature "dependency_file" looks like this:
From src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
"feature {",
" name: 'dependency_file'",
" flag_set {",
" action: 'assemble'",
" action: 'preprocess-assemble'",
" action: 'c-compile'",
" action: 'c++-compile'",
" action: 'c++-module-compile'",
" action: 'objc-compile'",
" action: 'objc++-compile'",
" action: 'c++-header-parsing'",
" action: 'clif-match'",
" expand_if_all_available: 'dependency_file'",
" flag_group {",
" flag: '-MD'",
" flag: '-MF'",
" flag: '%{dependency_file}'",
" }",
" }",
"}"),

But this is not regarded since action: 'assemble', will not produce a .d file, the flags above are not used, because of the hardcoded handling in bazel for ".S" and ".s"

So
action: 'assemble'",
action: 'preprocess-assemble'",

Does not work as expected I would say in this case.

Maybe we could change the actions to
assemble-s-files
assemble-S-files

Add an additional
assemble-asm-files

Remove the default for action: 'assemble' (or with the new name action: 'assemble-s-files'?

[emusand]

@hlopko, what is your opinion on the way forward?

You wrote in a comment to the pull request that it will be included in Bazel 0.22.0. A suggestion is that you submit this PR and that we close this ticket, but write a new ticket on making this configurable in CROSSTOOL.

[ehkloaj]

I work together with @emusand and @burkpojken.
After some investigation we have realized that the solution to this problem is more complex than it was suggested in #8055 and #6919.

The reason for this change is that we have realized that the list CppFileTypes.ASSEMBLER_WITH_C_PREPROCESSOR does not mean "run the preprocessor for these files", but instead "assume that these files will be preprocessed". Therefore just moving ".asm" to this list works in our internal environment because we are using a compiler that supports preprocessing of ".asm" files but users with compilers that are not supporting this (for example GCC and Clang) will experience build failures like this
ERROR: .../BUILD:3:1: error while parsing .d file: .../file.pic.d (No such file or directory). This is because bazel will add options "-MD -MF " to order the preprocessor to generate a ".d" file for ".asm" files but this will not be taken into account with compilers that are not preprocessing these files. Bazel will then expect a ".d" file for ".asm" which it can not find, hence the error. I am guessing this is what happened when the Google team ran internal tests, see #6919 (comment).

We are proposing a new approach to solving this where we add a new field in the toolchain definition, to tell bazel if the compiler will preprocess ".asm" files or not. What do you think of this?

[ehkloaj]

We think the right way forward is to add a list argument, preprocess_assembler_filename_extensions (or something shorter), to the function create_cc_toolchain_config_info with a default value of [".S"]. We then propagate the value of preprocess_assembler_filename_extensions to CppFileTypes to decide the value of the lists ASSEMBLER_WITH_C_PREPROCESSOR and ASSEMBLER.

This would allow us to specify that our compiler should preprocess .asm files while not breaking anything for users with other compilers since the default value suits them.

Adding the new argument seems easy enough. I think the hard part is to share the value of preprocess_assembler_filename_extensions with CppFileTypes so any suggestions on how this could be implemented would be much appreciated.

[meteorcloudy]

/cc Our C++ rule expert @oquenchil

[ehrtro]

The issue is that we still need to propagate the value of the list with what files that is to be preprocessed from CcToolchainConfigInfo to CppFileTypes. The value of this list is needed / requested in CppFileTypes from various other objects already before this list possibly could be propagated from CcToolchainConfigInfo, i.e there hasn't been any object of CcToolchainConfigInfo created before the information is needed in CppFileTypes. To only propagate the value is therefore not enough to fix this issue, a major redesign is required.

We think the right way forward is to add a list argument, preprocess_assembler_filename_extensions (or something shorter), to the function create_cc_toolchain_config_info with a default value of [".S"]. We then propagate the value of preprocess_assembler_filename_extensions to CppFileTypes to decide the value of the lists ASSEMBLER_WITH_C_PREPROCESSOR and ASSEMBLER.

This would allow us to specify that our compiler should preprocess .asm files while not breaking anything for users with other compilers since the default value suits them.

Adding the new argument seems easy enough. I think the hard part is to share the value of preprocess_assembler_filename_extensions with CppFileTypes so any suggestions on how this could be implemented would be much appreciated.

[SoftZL]

any progress ?our team have a lot of hand written .asm file

[emusand]

Unfortunately this work is parked for now. ehrtro, ehkloaj and I did a serious attempt to implement this in a proper way last summer, but realized that it was more work than we expected.

Now we are using our own local patch that just handles .asm files in the same way as .S files, that is we just adds ".asm" to the list of filename extensions that Bazel expects to be preprocessed. This works for us as our proprietary assembler always preprocesses .asm files, but it does not work when compiling with gcc or clang.

[meteorcloudy]

Sorry, we currently don't have the bandwidth to address this issue, but a contribution to fix is always welcome.

[SoftZL]

Thanks for reply @emusand @meteorcloudy
I will write my own rule first

[github-actions]

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 2+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

[github-actions]

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!