Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cil keeps inline and non-inline function with same name #120

Closed
jerhard opened this issue Nov 2, 2022 · 3 comments · Fixed by #124
Closed

Cil keeps inline and non-inline function with same name #120

jerhard opened this issue Nov 2, 2022 · 3 comments · Fixed by #124
Assignees
Labels
Milestone

Comments

@jerhard
Copy link
Member

jerhard commented Nov 2, 2022

As noted by @stilscher in goblint/analyzer#836 (comment), the merging in Cil does not behave correctly for inline functions.

I created a small example to test the open questions that @jerhard posted above. It seems that the merger is not behaving correctly. If merge_inline is disabled a function definition and an inline function definition with the same name but in a different translation unit do not get merged but are not renamed either. This is also the case when merge_inline is enabled but the function bodies are not the same. I will take a look at the merger and try to fix it, in a way that multiple function definitions either get a unique name or are merged.

@jerhard jerhard added the bug label Nov 2, 2022
@michael-schwarz
Copy link
Member

I think this might be in compliance to the pre-C99 GNU inlining semantics, we need to carefully think this through.

@stilscher stilscher self-assigned this Nov 2, 2022
@jerhard
Copy link
Member Author

jerhard commented Nov 2, 2022

Indeed, gnu89 allows for this. Also the meaning of extern inline and inline are swapped (except that extern inline in gnu89 allows redefinition, while c99 inline does not) in gnu89 and c99. One might have to handle this depending on the set language standard. I am not sure whether this is already handled in some way?

@michael-schwarz
Copy link
Member

There's an option enable or disable C99 mode, but I don't think it is consulted here. When I extended inline support I focussed on adding some cases that C99 allows, that GNU90 C did not, but it was en example-program driven effort, I never stopped to carefully consider all cases.

@sim642 sim642 added this to the 2.0.2 milestone Apr 5, 2023
sim642 added a commit to sim642/opam-repository that referenced this issue Sep 11, 2023
CHANGES:

* Rename `Rmtmps` to `RmUnused` (goblint/cil#135).
* Add option to add return statement to `noreturn` functions (goblint/cil#129).
* Fix empty `if`s being removed (goblint/cil#140).
* Fix `_Float128` support (goblint/cil#118, goblint/cil#119).
* Fix C11 `_Alignas` computation (goblint/cil#130).
* Fix renaming and merging of `inline` functions based on C standard (goblint/cil#120, goblint/cil#124).
* Fix `Pretty` not resetting all global state between calls (goblint/cil#133, goblint/cil#134).
* Fix `fundec` location in merger (goblint/cil#139).
* Fix `cilly` patcher (goblint/cil#128).
* Disable basename by default in parser.
nberth pushed a commit to nberth/opam-repository that referenced this issue Jun 18, 2024
CHANGES:

* Rename `Rmtmps` to `RmUnused` (goblint/cil#135).
* Add option to add return statement to `noreturn` functions (goblint/cil#129).
* Fix empty `if`s being removed (goblint/cil#140).
* Fix `_Float128` support (goblint/cil#118, goblint/cil#119).
* Fix C11 `_Alignas` computation (goblint/cil#130).
* Fix renaming and merging of `inline` functions based on C standard (goblint/cil#120, goblint/cil#124).
* Fix `Pretty` not resetting all global state between calls (goblint/cil#133, goblint/cil#134).
* Fix `fundec` location in merger (goblint/cil#139).
* Fix `cilly` patcher (goblint/cil#128).
* Disable basename by default in parser.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants