[ruby-core:118415] [Ruby master Bug#20601] Configuration flags are not properly propagated to assembler

[ruby-core:118415] [Ruby master Bug#20601] Configuration flags are not properly propagated to assembler

[vo.x (Vit Ondruch) via ruby-core]

Issue #20601 has been reported by vo.x (Vit Ondruch).

----------------------------------------
Bug #20601: Configuration flags are not properly propagated to assembler
https://bugs.ruby-lang.org/issues/20601

* Author: vo.x (Vit Ondruch)
* Status: Open
* ruby -v: ruby 3.3.2 (2024-05-30 revision e5a195edf6) [x86_64-linux]
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
Looking into #18061, one of the issues is that the `.S` files are not processed with the correct flags. For example to have the CET enabled, the `-fcf-protection` should be used to preprocess the `coroutine/amd64/Context.S`.

First I thought there is something wrong on Fedora side, therefore I have proposed to export the `ASFLAGS` [1]. However, as it turns out, `$(ASFLAGS)` are used by GNU make default rule and passed to `$(AS)`. And indeed, Ruby had historically override of this rule, but it does not do this anymore since:

https://github.com/ruby/ruby/commit/091422388e943de1e67ace6faac3d71ed08c14d2
https://github.com/ruby/ruby/commit/42575570a908aac979a80b89266804c4c688dd7c

As can be seen, while previously `$(AS)` was used to process the `.s` file, it was replaced by the compiler. This however means that the `.S` files are not preprocessed with the `$(CFLAGS)`, which contains `-fcf-protection`.


[1]: https://bugzilla.redhat.com/show_bug.cgi?id=2293107



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:118416] [Ruby master Bug#20601] Configuration flags are not properly propagated to assembler

[vo.x (Vit Ondruch) via ruby-core]

Issue #20601 has been updated by vo.x (Vit Ondruch).


This might be the medicine:

~~~patch
$ git diff
diff --git a/template/Makefile.in b/template/Makefile.in
index 2d232d7925..31e5ae5edb 100644
--- a/template/Makefile.in
+++ b/template/Makefile.in
@@ -449,7 +449,7 @@ $(srcdir)/enc/jis/props.h: enc/jis/props.kwd
 
 .$(ASMEXT).$(OBJEXT):
        @$(ECHO) assembling $<
-       $(Q) $(CC) $(ASFLAGS) -DSYMBOL_PREFIX=$(SYMBOL_PREFIX) -o $@ -c $<
+       $(Q) $(CC) $(ASFLAGS) -DSYMBOL_PREFIX=$(SYMBOL_PREFIX) $(CFLAGS) $(XCFLAGS) $(CPPFLAGS) $(COUTFLAG)$@ -o $@ -c $<
 
 .c.$(ASMEXT):
        @$(ECHO) translating $<
~~~

But I am not entirely convinced, if leaving the `$(ASFLAGS)` around is of any use.

----------------------------------------
Bug #20601: Configuration flags are not properly propagated to assembler
https://bugs.ruby-lang.org/issues/20601#change-108931

* Author: vo.x (Vit Ondruch)
* Status: Open
* ruby -v: ruby 3.3.2 (2024-05-30 revision e5a195edf6) [x86_64-linux]
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
Looking into #18061, one of the issues is that the `.S` files are not processed with the correct flags. For example to have the CET enabled, the `-fcf-protection` should be used to preprocess the `coroutine/amd64/Context.S`.

First I thought there is something wrong on Fedora side, therefore I have proposed to export the `ASFLAGS` [1]. However, as it turns out, `$(ASFLAGS)` are used by GNU make default rule and passed to `$(AS)`. And indeed, Ruby had historically override of this rule, but it does not do this anymore since:

https://github.com/ruby/ruby/commit/091422388e943de1e67ace6faac3d71ed08c14d2
https://github.com/ruby/ruby/commit/42575570a908aac979a80b89266804c4c688dd7c

As can be seen, while previously `$(AS)` was used to process the `.s` file, it was replaced by the compiler. This however means that the `.S` files are not preprocessed with the `$(CFLAGS)`, which contains `-fcf-protection`.


[1]: https://bugzilla.redhat.com/show_bug.cgi?id=2293107



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:118424] [Ruby master Bug#20601] Configuration flags are not properly propagated to assembler

[kjtsanaktsidis (KJ Tsanaktsidis) via ruby-core]

Issue #20601 has been updated by kjtsanaktsidis (KJ Tsanaktsidis).


I think we should do what we did for the pac-ret stuff, that Florian recommended, right? Run tests against CFLAGS in configure.ac, and then define special macros?

https://github.com/ruby/ruby/blob/93b19d56de64fdee790a96ddf96fcd08d889ac93/configure.ac#L1010-L1061

I'll have a bash at doing this tonight on my x86_64 machine.

----------------------------------------
Bug #20601: Configuration flags are not properly propagated to assembler
https://bugs.ruby-lang.org/issues/20601#change-108937

* Author: vo.x (Vit Ondruch)
* Status: Open
* ruby -v: ruby 3.3.2 (2024-05-30 revision e5a195edf6) [x86_64-linux]
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
Looking into #18061, one of the issues is that the `.S` files are not processed with the correct flags. For example to have the CET enabled, the `-fcf-protection` should be used to preprocess the `coroutine/amd64/Context.S`.

First I thought there is something wrong on Fedora side, therefore I have proposed to export the `ASFLAGS` [1]. However, as it turns out, `$(ASFLAGS)` are used by GNU make default rule and passed to `$(AS)`. And indeed, Ruby had historically override of this rule, but it does not do this anymore since:

https://github.com/ruby/ruby/commit/091422388e943de1e67ace6faac3d71ed08c14d2
https://github.com/ruby/ruby/commit/42575570a908aac979a80b89266804c4c688dd7c

As can be seen, while previously `$(AS)` was used to process the `.s` file, it was replaced by the compiler. This however means that the `.S` files are not preprocessed with the `$(CFLAGS)`, which contains `-fcf-protection`.


[1]: https://bugzilla.redhat.com/show_bug.cgi?id=2293107



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:118425] [Ruby master Bug#20601] Configuration flags are not properly propagated to assembler

[vo.x (Vit Ondruch) via ruby-core]

Issue #20601 has been updated by vo.x (Vit Ondruch).


Is something like that really needed? From my POV, the issue is that originally, there were `.s` files processed by Assembler, therefore the `$(ASFLAGS)` were needed. But now we have `.S` files pre-processed by compiler:

https://github.com/ruby/ruby/commit/e64f71f812324d098bed12ed68c2bc1d6e780c90
https://github.com/ruby/ruby/commit/42575570a908aac979a80b89266804c4c688dd7c

and therefore the same (or possibly extended) set of flags as for other `.c` files should be used. My understanding is that we should rather drop the `$(ASFLAGS)` usage.

----------------------------------------
Bug #20601: Configuration flags are not properly propagated to assembler
https://bugs.ruby-lang.org/issues/20601#change-108938

* Author: vo.x (Vit Ondruch)
* Status: Open
* ruby -v: ruby 3.3.2 (2024-05-30 revision e5a195edf6) [x86_64-linux]
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
Looking into #18061, one of the issues is that the `.S` files are not processed with the correct flags. For example to have the CET enabled, the `-fcf-protection` should be used to preprocess the `coroutine/amd64/Context.S`.

First I thought there is something wrong on Fedora side, therefore I have proposed to export the `ASFLAGS` [1]. However, as it turns out, `$(ASFLAGS)` are used by GNU make default rule and passed to `$(AS)`. And indeed, Ruby had historically override of this rule, but it does not do this anymore since:

https://github.com/ruby/ruby/commit/091422388e943de1e67ace6faac3d71ed08c14d2
https://github.com/ruby/ruby/commit/42575570a908aac979a80b89266804c4c688dd7c

As can be seen, while previously `$(AS)` was used to process the `.s` file, it was replaced by the compiler. This however means that the `.S` files are not preprocessed with the `$(CFLAGS)`, which contains `-fcf-protection`.


[1]: https://bugzilla.redhat.com/show_bug.cgi?id=2293107



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:118426] [Ruby master Bug#20601] Configuration flags are not properly propagated to assembler

[kjtsanaktsidis (KJ Tsanaktsidis) via ruby-core]

Issue #20601 has been updated by kjtsanaktsidis (KJ Tsanaktsidis).


OK, I poked at this a bit tonight. I have quite a few thoughts but they're kind of disorganised.

The short version of this is: I think you're right; we should pass the full set of `$CFLAGS $CPPFLAGS` when calling `$(CC)` on a .S file, and in fact I would go a step furthre and delete `$ASFLAGS` in `configure.ac` and `Makefile.in` entirely. It's late now but I'll try and put up a PR for this tomorrow or Friday.

The long verison...

--------
### Detecting control flow protection with configure tests

So I started on a PR to deal with `-fcf-protection` in intel context.S in the same way we deal with `-mbranch-protection` in arm context.S: https://github.com/ruby/ruby/compare/master...KJTsanaktsidis:ruby:ktsanaktsidis/fcf_prot.

This works, but it doesn't generate the build notes in the object file to declare that it has control flow protection. I was about to go and do that (in the same way we do it for ARM here https://github.com/ruby/ruby/blob/9aa62bda46bf7f9de91a95c31fa09dafd23def37/coroutine/arm64/Context.S#L109) but then I realised that the `<cet.h>` header being included _already does this_! It has something like this in it on my system

```
#ifdef __CET__
 .pushsection ".note.gnu.property", "a"
 // Lots of stuff ...
#endif
```

So including this header automatically emits the build notes, but _only if the CET macro is defined_, which will only be if `gcc context.S` is invoked as `gcc -fcf-protection=full context.S` instead. We could replicate the property stuff like we do for pac-ret, but it seems pretty silly to copy stuff out of the system headers like this.

If we do what you propose and invoke `gcc` as an assembler with `$CFLAGS`, this whole thing sorts itself out (and we can drastically simplify how we handle arm64 pac-ret too!)
---------


### Rant about GNU Make conventions

The conventions here are all kind of messed up. GCC (and compatible compiler drivers) support...

* Passing options related to C compilation directly to the compiler (e.g. `-fno-omit-frame-pointer`). These get passed to the `cc1` compiler process (I think)
* Passing options related to the preprocesser via `-Wp,` (e.g. you can spell `-DFOO=1` as `-Wp,-DFOO=1` if you wanted to). These get passed to `cpp` preprocessor
* Passing options related to the assembler via `-Wa,` (e.g. `-Wa,--generate-missing-build-notes=yes`). These get passed to the `gas` assembler
* Passing options related to the linker via `-Wl,` (e.g. `-Wl,-z,now`). These get passed to the `ld` linker

The GNU make conventions however are that...

* C files are turned into Object files by running `$(CC) $(CPPFLAGS) $(CFLAGS) -c $<`.
  - `$(CC)` is usually `gcc` or some compatible compiler driver.
  - `$CFLAGS` obviously should contain the C-compiler related flags.
  - `$CPPFLAGS` should contain flags related to the preprocessor. They _could_ be spelled with the `-Wp,` form, but the common preprocessor flags (`-I`, `-U`, `-D`, etc) are not required to be and usually aren't.
  - Internally, there is both a compilation step (C to assembly) and an assembling step (assembly to object), even though I guess gcc doesn't actually generate a textual form of the assembly unless you asked for it. You might want to provide some options to this inline assembler; you can achieve this by passing e.g. `-Wa,--defsym,foo=bar` to `gcc`. However, according to the make conventions, you'd put that in `$CFLAGS` (since that's what gets passed to gcc).
* Assembler files are turned into Object by running `$(AS) $(ASFLAGS) $<`.
  - `$(AS)` is usually `gas` or some actual assembler - NOT a compiler driver
  - `$ASFLAGS therefore has to be options spelled as they would be passed to `gas`; e.g. `--defsym foo=bar` instead of `-Wa,--defsym,foo=bar`
* Object files are turned into an executable by running `$(CC) $(LDFLAGS) $^`
  - Because `$LDFLAGS` are passed to `gcc`, they need to be spelled in the `-Wl,-z,now` format as GCC expects them, not in the `-z now` format that `ld` expects them

I guess this is all really kind of inconsistent:
  * `$CFLAGS` are passed to gcc (makes sense)
  * `$ASFLAGS` as passed to gas (makes sense)
  * `$LDFLAGS` are passed to gcc, not ld (???)
  * `$CPPFLAGS` are passed to gcc, not cpp (???)

But what you're saying is that Ruby does _not_ assemble with `$(AS) $(ASFLAGS)`, but rather by calling `$(CC) $(ASFLAGS)`. That would require that `$ASFLAGS` be spelt as `-Wa,...`, not as bare options like gas (and a "vanilla" GNU make setup) would expect! I think this is one of the reasons we should delete ASFLAGS - they're not passed to the assembler in the way that they "conventionally" should be, but rather more in a manner analagous to how LDFLAGS is handled.

----------------------------------------
Bug #20601: Configuration flags are not properly propagated to assembler
https://bugs.ruby-lang.org/issues/20601#change-108939

* Author: vo.x (Vit Ondruch)
* Status: Open
* ruby -v: ruby 3.3.2 (2024-05-30 revision e5a195edf6) [x86_64-linux]
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
Looking into #18061, one of the issues is that the `.S` files are not processed with the correct flags. For example to have the CET enabled, the `-fcf-protection` should be used to preprocess the `coroutine/amd64/Context.S`.

First I thought there is something wrong on Fedora side, therefore I have proposed to export the `ASFLAGS` [1]. However, as it turns out, `$(ASFLAGS)` are used by GNU make default rule and passed to `$(AS)`. And indeed, Ruby had historically override of this rule, but it does not do this anymore since:

https://github.com/ruby/ruby/commit/091422388e943de1e67ace6faac3d71ed08c14d2
https://github.com/ruby/ruby/commit/42575570a908aac979a80b89266804c4c688dd7c

As can be seen, while previously `$(AS)` was used to process the `.s` file, it was replaced by the compiler. This however means that the `.S` files are not preprocessed with the `$(CFLAGS)`, which contains `-fcf-protection`.


[1]: https://bugzilla.redhat.com/show_bug.cgi?id=2293107



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:118427] [Ruby master Bug#20601] Configuration flags are not properly propagated to assembler

[kjtsanaktsidis (KJ Tsanaktsidis) via ruby-core]

Issue #20601 has been updated by kjtsanaktsidis (KJ Tsanaktsidis).


Just one more thing: I was worried for a moment that trying to assemble with `$CFLAGS` would cause problems if you had some C-language specific things in cflags, but actually it seems to work fine - even if you have something like `-std=gnu11` or some such.

----------------------------------------
Bug #20601: Configuration flags are not properly propagated to assembler
https://bugs.ruby-lang.org/issues/20601#change-108940

* Author: vo.x (Vit Ondruch)
* Status: Open
* ruby -v: ruby 3.3.2 (2024-05-30 revision e5a195edf6) [x86_64-linux]
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
Looking into #18061, one of the issues is that the `.S` files are not processed with the correct flags. For example to have the CET enabled, the `-fcf-protection` should be used to preprocess the `coroutine/amd64/Context.S`.

First I thought there is something wrong on Fedora side, therefore I have proposed to export the `ASFLAGS` [1]. However, as it turns out, `$(ASFLAGS)` are used by GNU make default rule and passed to `$(AS)`. And indeed, Ruby had historically override of this rule, but it does not do this anymore since:

https://github.com/ruby/ruby/commit/091422388e943de1e67ace6faac3d71ed08c14d2
https://github.com/ruby/ruby/commit/42575570a908aac979a80b89266804c4c688dd7c

As can be seen, while previously `$(AS)` was used to process the `.s` file, it was replaced by the compiler. This however means that the `.S` files are not preprocessed with the `$(CFLAGS)`, which contains `-fcf-protection`.


[1]: https://bugzilla.redhat.com/show_bug.cgi?id=2293107



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/

[ruby-core:118430] [Ruby master Bug#20601] Configuration flags are not properly propagated to assembler

[vo.x (Vit Ondruch) via ruby-core]

Issue #20601 has been updated by vo.x (Vit Ondruch).


kjtsanaktsidis (KJ Tsanaktsidis) wrote in #note-4:
> and in fact I would go a step furthre and delete `$ASFLAGS` in `configure.ac` and `Makefile.in` entirely. It's late now but I'll try and put up a PR for this tomorrow or Friday.

I totally support removal of `$ASFLAGS`. I mulled over the idea, but I was not sure. But your analysis makes sense and provides the justification.

> but then I realised that the <cet.h> header being included already does this!

Correct. That is also my understanding.

> If we do what you propose and invoke gcc as an assembler with $CFLAGS, this whole thing sorts itself out (and we can drastically simplify how we handle arm64 pac-ret too!)

😍

----------------------------------------
Bug #20601: Configuration flags are not properly propagated to assembler
https://bugs.ruby-lang.org/issues/20601#change-108943

* Author: vo.x (Vit Ondruch)
* Status: Open
* ruby -v: ruby 3.3.2 (2024-05-30 revision e5a195edf6) [x86_64-linux]
* Backport: 3.1: UNKNOWN, 3.2: UNKNOWN, 3.3: UNKNOWN
----------------------------------------
Looking into #18061, one of the issues is that the `.S` files are not processed with the correct flags. For example to have the CET enabled, the `-fcf-protection` should be used to preprocess the `coroutine/amd64/Context.S`.

First I thought there is something wrong on Fedora side, therefore I have proposed to export the `ASFLAGS` [1]. However, as it turns out, `$(ASFLAGS)` are used by GNU make default rule and passed to `$(AS)`. And indeed, Ruby had historically override of this rule, but it does not do this anymore since:

https://github.com/ruby/ruby/commit/091422388e943de1e67ace6faac3d71ed08c14d2
https://github.com/ruby/ruby/commit/42575570a908aac979a80b89266804c4c688dd7c

As can be seen, while previously `$(AS)` was used to process the `.s` file, it was replaced by the compiler. This however means that the `.S` files are not preprocessed with the `$(CFLAGS)`, which contains `-fcf-protection`.


[1]: https://bugzilla.redhat.com/show_bug.cgi?id=2293107



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/