Skip to content

Commit 46311fa

Browse files
committed
fix: simplify dem_string_append logic and improve buffer handling
fix: improve dem_string_append handling for overlapping buffers perf: optimize dem_string_append overlap detection Improvements: 1. Add early return for empty strings (len == 0) 2. Fix overlap detection to check against ds->len instead of ds->cap - Only the USED portion [ds->buf, ds->buf + ds->len) can cause overlap - Checking against capacity was unnecessarily conservative 3. Fix boundary condition: use >= and <= instead of > and < - Adjacent non-overlapping ranges should use fast path This reduces unnecessary memory allocations in the common case where strings don't overlap with the destination buffer. fix: avoid undefined behavior on null pointer arithmetic in dem_string_append When ds->buf is NULL, performing pointer arithmetic (ds->buf + ds->cap) triggers UBSAN: 'applying zero offset to null pointer'. Fix by checking if ds->buf is NULL before doing pointer comparisons. If buf is NULL, we can safely append directly since there's no overlap.
1 parent 684bc62 commit 46311fa

File tree

1 file changed

+21
-8
lines changed

1 file changed

+21
-8
lines changed

src/demangler_util.c

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -284,14 +284,7 @@ char *dem_string_drain(DemString *ds) {
284284

285285
bool dem_string_append(DemString *ds, const char *string) {
286286
dem_return_val_if_fail(ds && string, false);
287-
size_t len = strlen(string);
288-
if (string > ds->buf + ds->cap || string + len < ds->buf) {
289-
return dem_string_append_n(ds, string, len);
290-
}
291-
char *string_copy = dem_str_ndup(string, len);
292-
bool result = dem_string_append_n(ds, string_copy, len);
293-
free(string_copy);
294-
return result;
287+
return dem_string_append_n(ds, string, strlen(string));
295288
}
296289

297290
bool dem_string_append_prefix_n(DemString *ds, const char *string, size_t size) {
@@ -315,13 +308,33 @@ bool dem_string_append_n(DemString *ds, const char *string, size_t size) {
315308
if (!size) {
316309
return true;
317310
}
311+
312+
// Check if we need to reallocate AND string points into our buffer
313+
// If both are true, we must copy first because realloc may move ds->buf
314+
bool needs_realloc = !dem_string_has_enough_capacity(ds, size);
315+
bool string_in_buffer = ds->buf && string >= ds->buf && string < ds->buf + ds->len;
316+
317+
char *string_copy = NULL;
318+
if (needs_realloc && string_in_buffer) {
319+
// Must copy before realloc to avoid use-after-free
320+
string_copy = dem_str_ndup(string, size);
321+
if (!string_copy) {
322+
return false;
323+
}
324+
string = string_copy; // Use the copy instead
325+
}
326+
327+
// Safe to proceed: either no realloc needed, or string is now external (copy)
318328
if (!dem_string_increase_capacity(ds, size)) {
329+
free(string_copy);
319330
return false;
320331
}
321332

322333
memcpy(ds->buf + ds->len, string, size);
323334
ds->len += size;
324335
ds->buf[ds->len] = 0;
336+
337+
free(string_copy); // No-op if NULL
325338
return true;
326339
}
327340

0 commit comments

Comments
 (0)