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

Support atomic types #688

Merged
merged 7 commits into from
Nov 1, 2024
Merged

Support atomic types #688

merged 7 commits into from
Nov 1, 2024

Conversation

schuessf
Copy link
Contributor

@schuessf schuessf commented Oct 16, 2024

This PR adds support for atomic types (_Atomic). To do so, I performed the following steps:

  • First the keyword is replaced by some artificial GCC attribute (__attribute__((atomic))) for our parser not to crash.
  • Whether we have an atomic type is then stored in the CType. This flag is only set explicitly in CPrimitive (based on the GCC attribute) and a CNamed is atomic iff the underlying type is atomic. All other types are currently not atomic. There can be actually an atomic pointer (see here), but we cannot parse the attribute properly at this location and thus don't support atomic pointers.
  • We don't check for data-races on atomic types (see DataRaceChecker).
  • We have to ensure that all operations on atomic variables are actually performed atomically. Therefore the translation of reads (see MemoryHandler::getReadCall), writes (see MemoryHandler::getWriteCall and CHandler::makeAssignment) and combined read-writes (e.g. ++, --, *=, ...) (see CHandler::makeAtomicAssignmentIfNecessary) was adapted.

@maul-esel
Copy link
Contributor

We have to ensure that all operations on atomic variables are actually performed atomically. Our translation seems to ensure this for read and write operations (I am not 100% sure though).

I think we should ensure this by adding atomic statements in Boogie.

This would also solve the current unsoundness on this branch when using the setting --rcfgbuilder.only.consider.context.switches.at.boundaries.of.atomic.blocks true aka the "assume no data race"-LBE.

@schuessf
Copy link
Contributor Author

I think we should ensure this by adding atomic statements in Boogie.

This would also solve the current unsoundness on this branch when using the setting --rcfgbuilder.only.consider.context.switches.at.boundaries.of.atomic.blocks true aka the "assume no data race"-LBE.

Thanks, that's a good point. With this settings, it actually makes a difference, whether we translate to a single Boogie statement st or atomic { st }.
So I guess, the only C expressions where the translation neeeds to be adapted for atomic types, are assignments (also sth. like +=) and reads (probably only for variables on the heap).

@schuessf schuessf force-pushed the wip/fs/atomic-types2 branch from 52f4372 to 25c038d Compare October 21, 2024 10:55
Copy link
Contributor

@maul-esel maul-esel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks for tackling this!

I've left some questions, and we should in particular also support other atomic operations such as += etc.

Copy link
Contributor

@maul-esel maul-esel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three small questions/requests left, otherwise it looks good. Thanks again for taking care of this feature!

to create AtomicStatement from a List / Stream of statements.
This method also avoid nesting of AtomicStatements
(should not make a difference on the CFG, but makes the Boogie code slightly nicer).
This adapts the translation for the following statements/expressions:
* reads of heap variables (`MemoryHandler::getReadCall`)
* writes of heap variables (`MemoryHandler::getWriteCall`)
* writes of non-heap variables (`CHandler::makeAssignment`)
* combined read-writes (e.g. `++`, `--`, `*=`, ...) (`CHandler::handleAtomicReadWrite`)
@schuessf schuessf force-pushed the wip/fs/atomic-types2 branch from 9101591 to 2b3c8f0 Compare November 1, 2024 11:34
@schuessf schuessf merged commit 2b3c8f0 into dev Nov 1, 2024
1 check was pending
@schuessf schuessf deleted the wip/fs/atomic-types2 branch November 1, 2024 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants