Repository navigation
Conversation
c17 warned "'cleanup' attribute directive ignored" and never ran the call, so systemd's _cleanup_* and glib's g_autoptr/g_autofree built and then leaked, never unlocked, never closed: silent wrong code. The parser resolves the function where the attribute is written and, in bind_declarator, builds fn(&var) through the ordinary call checks (checked_call, factored out of the postfix call parser). The linearizer (ir/linearize_cleanup.rs) keeps the calls in scope on a stack; one question, cleanups_kept_by(ScopeExit), decides what each exit runs, and run_cleanups_above emits them innermost first, in reverse declaration order: - falling out of a block, a for declaration or a statement expression, after the statement expression's value; - return, after the returned value is computed; a value the IR keeps in memory (complex, vector, aggregate wider than a register) is copied out first (outlive_cleanups), sret already copies; - break and continue, by the same depths VlaMark uses; - goto out of scope, forward or backward: JumpScopeWalk records each label's cleanup variables, so the goto runs them itself. A computed goto, asm goto, longjmp and exit run none, as in gcc; a jump into the scope is accepted, as in gcc. Diagnostics follow gcc 13: "cleanup argument not a function", "cleanup argument not an identifier", wrong argument count, and "'cleanup' attribute ignored" on static, file-scope, typedef, function, member and parameter declarations (silent on a block-scope extern). __has_attribute(cleanup) now answers 1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… values
Two defects behind `({ double _Complex v = 1.0; v; })` segfaulting on both
targets at every level:
- object_reads_as_address had no complex case, so naming a complex object
loaded its bits where every consumer expects the address of its
storage. Consumers that took the address of an lvalue themselves hid
it; a statement expression or comma operator passing the value through
dereferenced the bits.
- A statement expression whose value travels by address (complex,
vector, aggregate wider than a register) handed back the address of a
local whose lifetime then ended, so a second statement expression could
reuse the slot: f(({ struct S x = ..; x; }), ({ struct S y = ..; y; }))
passed y twice.
detach_value copies such a value into a function-lifetime temporary
before the block ends; outlive_cleanups (4b14866) now delegates to it,
so one copy protects the value from both the scope end and cleanups.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The four spellings (-funsigned-char, -fno-signed-char, -fsigned-char, -fno-unsigned-char) fell into the -f catch-all and were ignored with a warning, so a package asking for unsigned char on x86-64 silently got signed char. preprocess_args_from rewrites them to one internal option, --c17-plain-char, whose last occurrence wins as in gcc, and compile_main sets target.plain_char before anything reads it: the char type, __CHAR_UNSIGNED__, <limits.h> and #if character constants all follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`extern inline __attribute__((gnu_inline)) int f(void) { return 1; }`
followed by `int f(void) { return 0; }` returned 1 from -O1 up: both bodies
reached the module under one name and the inliner's lookup took the
first, the inline-only one. Module::add_function now keeps one function
per name (function_idx, the AppendIndex globals use): a real definition
replaces a same-name inline-only body, so every lookup -- inliner,
analysis, alias targets, dead-function removal -- sees the real one, as
gcc does.
A second defect behind the same symptom: the linearizer called a GNU
inline definition inline-only when any declaration of the name was
extern, where gcc and the parser look at the definition's own
specifiers only, so `extern int f(void); inline __attribute__((gnu_inline))
int f(void) {..}` emitted nothing and gnu89 orders failed to link. One
predicate, FunctionAttrs::gnu_inline_only, now answers both.
The parser also rejects what gcc rejects as "redefinition of 'g'": an
inline-only body after the real definition, and a second inline-only
body.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- -static was split by clap into -s -t -a ... ("unexpected argument
'-t'"); it is now a linker flag, and link_mode_flag makes any static
link -no-pie, as gcc 13 does for -static -pie; -static-pie gives a
static PIE.
- -mno-omit-leaf-frame-pointer / -momit-leaf-frame-pointer (Ubuntu 24.04
default CFLAGS) are accepted: both prologues always set up the frame
pointer.
- answer_driver_query: -dumpmachine (Target::gcc_triple, honours
--target), -dumpversion / -dumpfullversion, -print-multiarch in gcc's
one-dash spelling, and -print-file-name=, -print-search-dirs,
-print-libgcc-file-name, -print-multi-os-directory forwarded to the host
driver c17 links with (linkargs::host_driver, now shared).
- One GNUC_VERSION (7.5.0) drives __GNUC__/__GNUC_MINOR__/
__GNUC_PATCHLEVEL__/__VERSION__, the -dump answers and the -v banner.
- Bare -v prints a gcc-style banner ("gcc version 7.5.0 (c17 ...)") on
stderr and exits 0; with operands it stays verbose.
- @file response files expand first (respfile.rs, libiberty's
buildargv/expandargv rules: whitespace, quotes, backslashes, nesting,
unreadable file left literal; a cycle is an error), so the rewriter,
clap and linkargs::scan all see the expanded argv.
CPython's configure runs to completion with CC=c17.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Under -c, -S or -E, an operand c17 does not compile and that does not exist only warned "unrecognized file type" and exited 0, so a build naming a file it never made succeeded. As in gcc it is now "c17: error: <path>: linker input file not found: ..." and exit 1, while the remaining sources still compile. bypasses_linker decides which operands the driver checks itself: an unknown suffix always (it never reaches c17's link line), an object or archive only when nothing is linked (the linker reports one otherwise). An existing one still only warns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Debian's dpkg-buildflags pass -ffile-prefix-map=<builddir>=. so objects do not embed the build path; c17 ignored it, so DWARF and __FILE__ carried the absolute directory and objects differed between build trees. prefix_map.rs holds one rule, PrefixMap::apply, measured against gcc 13: the last matching option on the command line wins (not the longest), OLD is a plain string prefix, and OLD=NEW splits at the last '='. The three spellings ride one hidden clap list so their order survives parsing; -ffile-prefix-map feeds both maps at its position. pipeline.rs applies the debug map to DW_AT_name, comp_dir and every .file path; the preprocessor applies the macro map to __FILE__ and __BASE_FILE__ (and so assert(), #line names, headers and .S files), not to -E linemarkers. Malformed values are gcc's errors. __BASE_FILE__ now goes through the literal payload helper like __FILE__, fixing non-ASCII paths. The same source compiled with -g -ffile-prefix-map=<dir>=. in two directories now gives identical .s and .o. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A source path holding '"' broke the build ("junk at end of line"): .file
directives and DWARF names were written unescaped. One assembler escaping
rule, escape_bytes in arch/codegen.rs, now serves .ascii/.asciz data
(escape_string) and paths (escape_path): .file lines on every target and
DW_AT_name/DW_AT_comp_dir.
On the C side, one escape_c_string spells text inside a string literal
for __FILE__/__BASE_FILE__, the # stringify operator (replacing its two
copies of the loop) and the three -E linemarker writers. #line and
linemarker names are now decoded through the escape parser
(decode_string_spelling), as gcc does, so `#line 1 "a\\b"` is the file
a\b and c17 -E output round-trips through c17.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
c17 ignored ifunc with a warning and left the symbol undefined, so
glibc-style runtime dispatch failed to link. An ifunc is now an alias of
another form: SymbolAttrs.alias carries AliasAttr { target, form:
AliasForm::{Alias, Ifunc} } through declare_alias / resolve_aliases into
ir::SymbolAlias, and the one emission path adds
`.type f, @gnu_indirect_function` before `.set f, resolver`
(SymbolType::GnuIndirectFunction). The name joins extern_symbols, so its
address loads through the GOT and calls go through the PLT, as gcc
emits; the resolver stays alive as an alias target.
gcc 13's diagnostics: undefined resolver, resolver that is a variable,
redefinition, weak + ifunc, a resolver returning a non-pointer (error)
or another pointer type (warning), ifunc on a variable ignored with a
warning; Mach-O has no indirect functions and is an error.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`void *` against a function pointer is a constraint violation gcc accepts
silently as a GNU extension and flags only under -pedantic; c17 warned
by default (its own -Wfunction-pointer-conv group), which broke -Werror
builds of the everyday `fp f = dlsym(h, "x");`.
diag::Pedantic is now the one owner: -pedantic / -Wpedantic turn it on,
-pedantic-errors makes its diagnostics errors, -Wno-pedantic turns it off,
folded in command-line order as gcc does; diag::pedwarn emits through it.
The driver passes -pedantic(-errors) as -W names so the order survives;
the dead --pedantic field and the invented -Wfunction-pointer-conv group
are gone.
Under the switch, with gcc 13's wording, the pair is flagged in all seven
contexts: return, initialization, assignment, argument ("passing argument
N of 'f'"), the conditional operator, an explicit cast (any object
pointer, null constants excepted) and a comparison -- the last two were
missing. One predicate, TypeTable::pointees_pair_function_with_void,
serves the implicit contexts. `int f(...)` moves under the switch too.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Runtime CPU dispatch (zlib-ng, xxhash, pixman) compiles one function for
a higher ISA; c17 ignored both attributes with a warning, so the
function never got the higher ISA's instructions and -Werror builds
failed.
- target: a per-function ISA. IsaRequest (arch= set plus feature edits)
is applied by X86Isa::with_request through the existing isa_edit /
IsaSet; Linearizer::function_isa sets ir::Function::isa, and the two
readers -- arch::simd::native (now given the ISA) and the x86-64
backend (CodeGenBase::isa) -- use it. Feature macros stay at the
unit's level, as in gcc. Attributes on a prototype reach the
definition.
- The inliner refuses to inline a body into a caller whose ISA does not
cover it (X86Isa::includes), as gcc does in both directions.
- target_clones (ir/target_clones.rs): the body is linearized once per
version under gcc's names (sum.default, sum.sse4_2, ...), local, with
static locals shared; a resolver in IR reads __cpu_model /
__cpu_features2 at gcc's bits and priority order (cpu_feature_location,
shared with __builtin_cpu_supports); the name becomes an ifunc alias
bound to the resolver, which is .weak in gcc's comdat section.
- c17's SIMD ceiling is SSE4.2: an ISA above it is named in a warning and
ignored, and such a clone is not built. aarch64 accepts arch=/cpu=/
tune=/+ext and rejects the rest, and rejects clone lists ("target does
not support function version dispatcher"); Mach-O builds the default
version under the plain name. Diagnostics use gcc 13's wording.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Statement-expression macros (`({ __auto_type _a = (a); ... })`) failed to
parse. __auto_type is now a type specifier in the one specifier loop,
recorded as a data type so `__auto_type int` gets 6.7.2p2's error, and
allowed only in declarations: a new SpecContext::Parameter separates
parameters from declarations so the loop can refuse it there in gcc's
words.
The marker is a type, not a flag: DeclSpecs.base is DeclBase::{Given,
Inferred}. infer_auto_type checks gcc's rules in gcc's order (single
declarator, plain identifier, initializer present, not a typedef),
parses the initializer before the name is declared -- so it is not in
scope there -- and gives the object the initializer's lvalue-converted
type (arrays and functions decay, qualifiers drop) plus the qualifiers
written with it. The initializer is parsed and checked once
(settle_initializer, now shared with ordinary declarations).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A statement-expression macro that jumps to its own label could not be
expanded twice in one function: labels were function-wide by name and
__label__ was unknown.
A label's identity is now a type, LabelId { name, scope:
LabelScope::{Function, Local(n)} }, carried by every label definition,
goto, &&label and asm goto label. The parser keeps a stack of local
label scopes, pushed around every block, function body and statement
expression; `__label__ a, b;` at a block's head declares labels numbered
uniquely in the unit, and resolve_label takes the innermost declaration
or else the function's label. The linearizer -- label maps, jump-scope
walk, cleanup label lists -- is keyed by LabelId, so VLA and cleanup
jump rules hold for local labels; diagnostics still print the spelling.
gcc 13's diagnostics: used but not defined outside its block, placement
after a declaration or statement, file scope, duplicate label and
duplicate label declaration.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The change set spans high-risk compiler internals (diagnostics behavior change, preprocessor file-name mapping, and codegen-adjacent test coverage) and the full PR is larger than the reviewable slice, so final human review is warranted.
Review effort: Balanced
Findings: None
What changed in this PR
This PR ("CC fixes") applies a set of correctness and cleanup changes to the cc crate (the in-tree C17 compiler). The visible portion centralizes the function-pointer↔void * diagnostic into reusable TypeTable helpers, consolidates the GCC version claim into a single GNUC_VERSION constant shared by the predefined macros, wires -fmacro-prefix-map through to __FILE__/__BASE_FILE__ with proper C-string escaping/decoding, and adds regression tests for statement-expression values, __label__ local labels, prefix-map rewriting, and file-name escaping.
Changes:
- Centralize function-pointer/
void *pairing (pointees_pair_function_with_void) and pointer-type formatting (format_pointer_to) intypes.rs; remove the obsoleteFUNCTION_POINTER_CONVconstant. - Introduce
GNUC_VERSIONas the single source of the claimed GCC release and apply-fmacro-prefix-mapto file-name macros via newescape_c_string/decode_string_spelling/file_macro_payloadhelpers. - Add regression tests (statement-expression complex/struct/vector values,
__label__, macro-prefix-map, file-name escaping).
| File | Description |
|---|---|
| cc/types.rs | Adds format_pointer_to and pointees_pair_function_with_void; removes FUNCTION_POINTER_CONV; refactors assignment_fault; adds tests. |
| cc/token/lexer.rs | Adds escape_c_string and its inverse decode_string_spelling for C-string payloads/file names. |
| cc/token/preprocess.rs | Adds GNUC_VERSION constant, macro_prefix_map field, and file_macro_payload; threads the prefix map through configs. |
| cc/token/preprocess_macro.rs | Uses escape_c_string for #/#@ stringification and file_macro_payload for __FILE__/__BASE_FILE__. |
| cc/token/preprocess_directive.rs | Decodes #line/linemarker names via decode_string_spelling (interprets escapes). |
| cc/token/test_preprocess.rs | Tests for GNUC_VERSION macros, prefix-map rewriting, and file-name escaping. |
| cc/tests/misc/stmt_expr.rs | Regression tests for statement-expression values (complex/struct/vector) across -O0/-O2 and aarch64. |
| cc/tests/misc/mod.rs | Registers new auto_type, cleanup_attr, and local_label test modules. |
| cc/tests/misc/local_label.rs | New test for GNU __label__ block-scoped labels. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…s once Two findings from review of 3fb923d: - constructor/destructor on a target_clones function were silently dropped, so neither ran. As in gcc 13, name.default is registered in .init_array/.fini_array, priority kept. One place now divides a definition's attributes among the versions, the resolver and the ifunc name (Division::of, table in its doc comment), replacing two ad hoc copies: section and the code-generation attributes on every version, used on versions and resolver, visibility on the ifunc name, weak and alias dropped. - The body is lowered once per version, so every source diagnostic printed once per version. diag::each_once gives each distinct diagnostic once (and counts it once) around the version loop; every per-version check still runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
macOS CI failed prefix_map_file_makes_builds_reproducible: identical .s, different .o. Apple's assembler is clang's integrated one, and when it writes a DWARF 5 line table it records its own working directory as include_directories[0] -- c17 never told it the mapped directory. GNU as does not, so Linux passed. Reproduced with llvm-mc -dwarf-version=5 in two directories; -fdebug-prefix-map makes the objects identical, which is how clang itself keeps -ffile-prefix-map builds reproducible. One AssemblerCommand builder now serves both assembler call sites (compiled C and user .s/.S): on macOS with a debug prefix map it runs the host driver, `cc -c -x assembler IN -o OUT [-g] -fdebug-prefix-map=OLD=NEW...` in command-line order; every other case is the old `as [-g] -o OUT IN`. linkargs::HOST_DRIVER names the driver in one place. Verified by the llvm-mc proxy and unit tests on the command line; the macOS run itself is CI's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fa8c5a5 deliberately departed from gcc 13 in two places; match it instead, so a library built with c17 exports what a gcc build exports (Debian's symbols files are made from gcc builds): - section("x"): the non-default versions go in x, name.default stays in .text. - visibility("hidden"): dropped; the indirect symbol is exported (IFUNC GLOBAL DEFAULT), as gcc 13 emits it. No gcc bug report or later fix for this was found. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.