ocaml / ocaml

The core OCaml system: compilers, runtime system, base libraries
https://ocaml.org
Other
5.19k stars 1.06k forks source link

[runtime] Introduce a platform-independent header for portable CFI/DWARF constructs #13119

Closed dustanddreams closed 2 weeks ago

dustanddreams commented 2 weeks ago

This PR introduces a common header for use by .S runtime files to get cfi directive macros, and machine-independent DWARF values. This will help add cfi support to missing platforms and reduces duplication.

I am not sure which copyright attribution to use for the new header - it looks like most of these defines have been introduced in 2012, but without changes to the legalese.

gasche commented 2 weeks ago

I would consider crediting the original author in 2012 (do you have the commit hash?), blindly copying the formulation of copyright headers for that person if they are already listed somewhere.

Dumb questions:

dustanddreams commented 2 weeks ago

I would consider crediting the original author in 2012 (do you have the commit hash?), blindly copying the formulation of copyright headers for that person if they are already listed somewhere.

Unfortunately due to tree layout changes it is painful to follow the history and I'm not sure I'd find the authors of everything (cfi directives and dwarf defines).

Dumb questions:

* if you are in `runtime`, why use `../runtime` at the beginning of the include path?

Consistency with other #include stanzas in the runtime/*.S files.

* for me `asm.h` is a bit obscure, have you considered having `cfi.h` and `dwarf.h`? (This is not a strong opinion / request, maybe you find `asm.h` very reasonable and in this case feel free to leave things as-is)

I am not sure this is worth splitting into multiple files. Also, after some more homogeneization work, this file could also contain most of the defines (such as FUNCTION, END_FUNCTION) for ELF platforms, allowing machine-dependent files to only provide macOS and windows overrides.

gasche commented 2 weeks ago

I blame Xavier, Bart, Tom: 2eecf2d4c0933f64bcfa3e620f031497166db338, 2eecf2d4c0933f64bcfa3e620f031497166db338, 74e9e0984e93efc560423acfa8220f1b885fc5a0 . I thus propose the following copyright lines (reflecting the affiliation at the time the code was written):

/*             Xavier Leroy, projet Gallium, INRIA Rocquencourt           */
/*             Bart Jacos, KU Leuven                                      */
/*             Tom Kelly, OCaml Labs Consultancy, UK                      */
dustanddreams commented 2 weeks ago

Thanks.

jmid commented 2 weeks ago
/*             Bart Jacos, KU Leuven                                      */

Nit: Isn't it Jacobs? :thinking: https://distrinet.cs.kuleuven.be/people/BartJacobs

gasche commented 2 weeks ago

Indeed, this was a typo on my part.

dustanddreams commented 2 weeks ago

Woopsie. Updated.