checkedc / checkedc-llvm-project

This was a fork of Checked C clang used from 2021-2024. The changes have been merged into the original Checked C clang repo, which is now at https://github.com/checkedc/checkedc-clang.
https://www.checkedc.org
13 stars 19 forks source link

Fix issue 1204 assertion when compiling openssh-portable source code #1228

Closed dtarditi closed 9 months ago

dtarditi commented 10 months ago

This fixes issue #1204, when an assertion in generating LLVM IR occurred when compiling open-ssh-portable source code. The assertion occurred because of an unintended modification to an ImplicitCastExpr by the TreeTransform class. This occurs when we use TreeTransform to do an expression substitution, where we substitute some expression E1 for E2. This is done as part of tracking equivalent expressions during check of bounds declarations. If E1 was an ImplicitCastExpr, it was accidentally modified.

The root cause was that TreeTransform calls semantic actions, which eventually call Sema::ImpCastExprToType. This function creates an implicit cast using some expression E as the child of the cast. However, if E is already an implicit cast, it merges the two implicit casts expressions, side-effecting E. The fix is to generate a new combined implicit cast instruction, instead of side-effecting E. That way TreeTransform is non-side-effecting when existing expressions are introduced as part of the AST tree transform. This can cause the existing ImplicitCastExpr E to be unused, but that is not a problem, because we drop pointers to expressions all over the place.

For other uses of TreeTransform in clang, the existing behavior has been fine because TreeTransform disregards implicit cast expressions. It assumed that the semantic actions generated new implicit casts where necessary, so side-effecting one was OK.