Skip to content

Harden malloc implementation #226

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

Merged
merged 1 commit into from
Aug 8, 2025
Merged

Harden malloc implementation #226

merged 1 commit into from
Aug 8, 2025

Conversation

jserv
Copy link
Collaborator

@jserv jserv commented Aug 2, 2025

The original malloc was development-only with critical flaws:

  • No alignment
  • Overflow vulnerability (calloc multiplication)
  • Null pointer crashes (cleanup functions)
  • Complex allocation logic

New implementation adds:

  • 8-byte alignment for cache optimization
  • Bounds checking preventing integer overflow
  • Null-safe memory management operations

Summary by Bito

This pull request enhances the malloc implementation by fixing alignment issues, preventing overflow vulnerabilities, and addressing null pointer crashes. It introduces 8-byte alignment for cache optimization, bounds checking for integer overflow, and null-safe memory management, improving reliability and performance.

@jserv jserv requested review from ChAoSUnItY and DrXiao August 2, 2025 21:28
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Aug 3, 2025
@jserv jserv force-pushed the improve-alloc branch 2 times, most recently from 8adfe48 to 3802292 Compare August 5, 2025 14:36
@DrXiao
Copy link
Collaborator

DrXiao commented Aug 5, 2025

I fetched the updated commits on my beaglebone black and rebuilt shecc. However, when using 3802292, the build process encountered a segfault issue during the stage 2 compiler build.

debian@BeagleBone:~/shecc$ make
env printf "ARCH=arm" > .session.mk
Target machine code switch to arm
Warning: missing packages: dot jq
Warning: Please check package installation
  CC+LD out/inliner
  GEN   out/libc.inc
  CC    out/src/main.o
  LD    out/shecc
  SHECC out/shecc-stage1.elf
  SHECC out/shecc-stage2.elf
make: *** [Makefile:129: out/shecc-stage2.elf] Segmentation fault

@jserv
Copy link
Collaborator Author

jserv commented Aug 5, 2025

I fetched the updated commits on my beaglebone black and rebuilt shecc. However, when using 3802292, the build process encountered a segfault issue during the stage 2 compiler build.

See if commit dc66c25 helps.

@jserv
Copy link
Collaborator Author

jserv commented Aug 6, 2025

For further improvements, it depends on #230 for richer expressions.

@DrXiao
Copy link
Collaborator

DrXiao commented Aug 6, 2025

See if commit dc66c25 helps.

After fetching this commit, the build still fails when compiling the stage 2 compiler on my beaglebone board.

@DrXiao
Copy link
Collaborator

DrXiao commented Aug 6, 2025

Since several pull requests have been merged recently, it may be necessary to rebase the latest master branch again for the build process to succeed on the beaglebone board.

If the build still fails on the beaglebone black after rebasing, we should review the proposed changes or the previously merged commits to identify any potential issues.

@DrXiao
Copy link
Collaborator

DrXiao commented Aug 6, 2025

I have fetched the latest master branch and build shecc on my board again. Unexpectedly, the build also fails due to a segfault. Next, I will use git-bisect to find the root cause.

@jserv
Copy link
Collaborator Author

jserv commented Aug 6, 2025

I have fetched the latest master branch and build shecc on my board again. Unexpectedly, the build also fails due to a segfault. Next, I will use git-bisect to find the root cause.

@ChAoSUnItY, can you confirm with Raspberry Pi + 32-bit Raspbian/Debian?

@ChAoSUnItY
Copy link
Collaborator

ChAoSUnItY commented Aug 6, 2025

can you confirm with Raspberry Pi + 32-bit Raspbian/Debian?

I've confirmed the following context:

  1. It indeed crashes when building stage 1 compiler.
  2. It crashes when parsing the parameter shift_type shift in declaration of function __srl_amt defined in arm.c.
  3. When parsing it, the type is found and it returns a nonnull pointer, but GDB indicates that the pointer it returns is not accessable (this occurs on line 721), which triggers segment fault.

    shecc/src/parser.c

    Lines 704 to 724 in dc66c25

    void read_full_var_decl(var_t *vd, int anon, int is_param)
    {
    char type_name[MAX_TYPE_LEN];
    int find_type_flag = lex_accept(T_struct) ? 2 : 1;
    lex_ident(T_identifier, type_name);
    type_t *type = find_type(type_name, find_type_flag);
    if (!type) {
    printf("Could not find type %s%s\n",
    find_type_flag == 2 ? "struct " : "", type_name);
    abort();
    }
    vd->type = type;
    /* Inherit pointer level from typedef */
    if (type->ptr_level > 0)
    vd->is_ptr = type->ptr_level;
    read_inner_var_decl(vd, anon, is_param);
    }

Judging from above information, I suspect this could be memory corruption done by incorrect memory handing.

@jserv
Copy link
Collaborator Author

jserv commented Aug 7, 2025

Judging from above information, I suspect this could be memory corruption done by incorrect memory handing.

Check #237

@DrXiao
Copy link
Collaborator

DrXiao commented Aug 7, 2025

As shown in #205 (comment) , the hybrid allocation strategy seems to have no significant impact on reducing memory use.

After reviewing the current source code of shecc, I noticed that most of malloc() calls are used to allocate large objects, so the proposed changes don't significantly improve the memory efficiency, since small objects are already allocated space using hashmap or the arena allocator.

@jserv jserv changed the title Improve malloc with hybrid allocation strategy Harden malloc implementation Aug 8, 2025
The original malloc was development-only with critical flaws:
- No alignment
- Overflow vulnerability (calloc multiplication)
- Null pointer crashes (cleanup functions)
- Complex allocation logic

New implementation adds:
- 8-byte alignment for cache optimization
- Bounds checking preventing integer overflow
- Null-safe memory management operations
@jserv jserv merged commit 498da37 into master Aug 8, 2025
12 checks passed
@jserv jserv deleted the improve-alloc branch August 8, 2025 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants