Skip to content

Commit 492bcb5

Browse files
Agent Userclaude
andcommitted
converter: fix the printf argument/cast misalignment, and refuse loudly on a structured binding
printf casts were taken from the wrong slot, which is why `printf("%c")` printed a numeric value -- a symptom recorded in AGENT-COMMON for a while and now root-caused. Two independent bugs in the same alignment: 1. `if (types[j]) StrCat(kAs, types[j++]);` advanced `j` ONLY on a non-null entry. `types` is parallel to the VARIADIC ARGUMENTS -- printf2fmt pushes one entry per conversion that consumes an argument, and a null entry means "pass through, no cast" -- so every pass-through conversion PINNED `j` and the next cast-needing conversion took its cast from the wrong slot. Measured on `printf("%d %c\n", 65, 'B')`: the `%d` entry is null, `j` stayed 0, and the `u8 as char` cast from the `%c` slot was applied to the FIRST argument. It compiled and printed the wrong values. The index is now `i - first`, with a bounds check so more arguments than conversions gets no cast rather than an out-of-bounds read. 2. `case '%'` pushed a `types` entry, but `%%` is a literal percent and consumes NO argument, so it shifted every later argument by one on its own. 73cfd28 had made (1) more reachable: its survey arm pushes a null entry for an unknown specifier, so `%f` followed by `%c` under --survey hits it too. Verified byte-exact against clang after the fix, both models: C++ `65 B`, unsafe MATCH, refcount MATCH; and the `%%` case `100% B`. ReportUnsupportedStructuredBinding: the same NDEBUG silent-omission class as 73cfd28's exception fix, confirmed by measurement rather than by analogy. With the pre-fix release binary a TU containing `auto [x, y] = p;` exited rc=0 and the emitted Rust contained the following `println!` referring to `x` and `y` while the declaration itself had VANISHED -- VisitVarDecl (converter.cpp:678) returns false having emitted no text. Now report_fatal_error naming the construct, its bindings and the location (rc=134); the pre-existing survey arm still records and continues (rc=0, count=1). Ranking note for the remaining asserts: standard Cpp2Rust#1 (NDEBUG-erased non-termination) now has NO candidate. All 30 `assert(0` sites were scanned mechanically for "last statement of a loop body with a non-advancing cursor" and the printf loop fixed in 73cfd28 was the only one. Remaining sites are Cpp2Rust#2 (silent omission) and Cpp2Rust#3 (UB). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 8605102 commit 492bcb5

2 files changed

Lines changed: 35 additions & 7 deletions

File tree

‎cpp2rust/converter/converter.cpp‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3880,8 +3880,14 @@ void Converter::ReportUnsupportedStructuredBinding(
38803880
survey::Record(survey::GapKind::kUnsupportedConstruct, detail, loc);
38813881
return;
38823882
}
3883-
llvm::errs() << "unsupported " << detail << " at " << loc << '\n';
3884-
assert(0 && "unsupported structured binding (DecompositionDecl)");
3883+
// SILENT OMISSION under NDEBUG, which is the build everyone ships: the assert
3884+
// compiled to nothing, this returned, VisitVarDecl (converter.cpp:678)
3885+
// returned false having emitted NO TEXT for the declaration, and the process
3886+
// exited 0 -- so the `.rs` simply lacked the variable while every later use of
3887+
// its bindings referred to a name that was never declared. Same class as
3888+
// ReportUnsupportedException (73cfd28); refuse loudly instead, naming the
3889+
// construct, its bindings and the source location.
3890+
llvm::report_fatal_error(llvm::Twine("unsupported ") + detail + " at " + loc);
38853891
}
38863892

38873893
void Converter::ReportUnmappedSystemType(const clang::RecordDecl *decl) {

‎cpp2rust/converter/models/converter_refcount.cpp‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -999,7 +999,11 @@ static std::vector<const char *> printf2fmt(std::string &format,
999999
pos += 2;
10001000
continue;
10011001
case '%':
1002-
types.emplace_back();
1002+
// `%%` is a LITERAL percent: it consumes NO argument, so it must NOT push
1003+
// a `types` entry. `types` is indexed by ARGUMENT (see ConvertPrintf), and
1004+
// an entry here shifted every later argument's cast by one -- measured on
1005+
// `printf("100%% %c\n", ch)`, where the `%c` cast landed on the `%%` slot
1006+
// and the char printed as its numeric value.
10031007
format.replace(pos, 2, "%");
10041008
pos += 2;
10051009
continue;
@@ -1127,12 +1131,30 @@ void ConverterRefCount::ConvertPrintf(clang::CallExpr *expr) {
11271131
format, expr->getBeginLoc().printToString(ctx_.getSourceManager()));
11281132
StrCat(format);
11291133

1130-
unsigned j = 0;
1131-
for (unsigned i = is_fprintf + 1, e = expr->getNumArgs(); i < e; ++i) {
1134+
// INVARIANT: `types` is parallel to the VARIADIC ARGUMENTS -- printf2fmt
1135+
// pushes exactly one entry per conversion that consumes an argument, and a
1136+
// NULL entry means "pass this argument through with no cast". So the cast for
1137+
// argument `i` is `types[i - first]`, nothing else.
1138+
//
1139+
// This used to read `if (types[j]) StrCat(kAs, types[j++]);`, which advanced
1140+
// `j` ONLY on a non-null entry: every pass-through conversion PINNED `j`, and
1141+
// the next cast-needing conversion then took its cast from the WRONG SLOT and
1142+
// applied it to an argument it does not belong to. Measured on
1143+
// `printf("%d %c\n", 65, 'B')`: the `%d` entry is null, so `j` stayed 0 and
1144+
// the `u8 as char` cast from the `%c` slot was applied to the FIRST argument,
1145+
// emitting `println!("{} {}", 65 as u8 as char, 'B' as u8)` -- prints "A 66"
1146+
// where C prints "65 B". It compiled and printed the wrong values, i.e. silent
1147+
// wrongness. 73cfd28 made it more reachable: the survey arm of printf2fmt now
1148+
// pushes a null entry for an unknown specifier, so `%f` followed by `%c`
1149+
// under --survey hits it too.
1150+
const unsigned first = is_fprintf + 1;
1151+
for (unsigned i = first, e = expr->getNumArgs(); i < e; ++i) {
11321152
StrCat(token::kComma);
11331153
Convert(expr->getArg(i));
1134-
if (types[j])
1135-
StrCat(keyword::kAs, types[j++]);
1154+
// A variadic argument with no conversion left to describe it (more args than
1155+
// conversions) gets no cast rather than an out-of-bounds read.
1156+
if (unsigned j = i - first; j < types.size() && types[j])
1157+
StrCat(keyword::kAs, types[j]);
11361158
}
11371159
StrCat(')');
11381160
}

0 commit comments

Comments
 (0)