Skip to content
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

Check for invalid digits when parsing octal constants #153

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

DrXiao
Copy link
Collaborator

@DrXiao DrXiao commented Sep 15, 2024

After introducing the pull request (#151 ), I use the following code to test the parser:

/* test.c */
int main() {
    int a = 03, b = 01118, c = 091;
    printf("%d %d %d\n", a, b, c); 
    return 0;
}
$ qemu-arm out/shecc-stage2.elf -o test test.c
$ qemu-arm test
3 592 73

Obviously, the octal numbers (01118 and 091) are invalid but the parser doesn't check whether they are valid or not.

Thus, the pull request improves the parser to enhance the validation check.

src/parser.c Outdated
@@ -634,7 +634,10 @@ void read_numeric_param(block_t *parent, basic_block_t *bb, int is_neg)
} while (is_hex(token[i]));
} else { /* octal */
do {
c = token[i++] - '0';
c = token[i++];
if (c > '7' || c < '0')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check for c < '0' is redundant because the loop will exit as soon as is_digit() in line 642 returns false.
For example, consider the case int a = 0.; funct(a);, where the ASCII value of '.' is less than '0'.
On the first iteration, c = token[i++] assigns c the value '0' and increments i to 1.
Then, the check in the while loop at line 642 is_digit(token[i = 1]) will become is_digit('.') and return false, causing the loop to exit.
As a result, the check for c < '0' is never triggered when the comming digit is less then '0'.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fennecJ , yes, here doesn't need to check for c < '0'. Thank you for your feedback, I will improve it later.

@jserv jserv requested a review from ChAoSUnItY September 15, 2024 12:39
Originally, the parser had the ability to parse octal numbers
but did not check whether the digits were valid. Therefore, this
commit improves the implementation to include the validation check.
@ChAoSUnItY
Copy link
Collaborator

ChAoSUnItY commented Sep 15, 2024

Please also add your code to the test suite for code validation.

@DrXiao
Copy link
Collaborator Author

DrXiao commented Sep 15, 2024

The proposed changes let shecc generate a compilation error message when parsing invalid octal numbers.

However, originally, tests/driver.sh only compiles valid C programs and checks the exit code or standard output, and the test suite has never compiled any invalid C program before.

This is the first instance of adding an invalid C program, which includes some invalid octal constants, to test/driver.sh to check whether (native) shecc generates the error message or not.

Thus, I will add a new function called try_compile_error() to the test suite, and the function will accept an invalid C program and then check if the exit code returned by shecc is a non-zero value. Eventually, the test suite will have the ability to test invalid C programs.

function try_compile_error() {
    local input=$(cat)

    local tmp_in="$(mktemp --suffix .c)"
    local tmp_exe="$(mktemp)"
    echo "$input" > "$tmp_in"
    "$SHECC" -o "$tmp_exe" "$tmp_in"
    local exit_code=$?

    if [ 0 == $exit_code ]; then
        echo "Error: compilation is passed"
        exit 1
    fi
}


# ...

try_compile_error << EOF
int main() {
    int a = 03, b = 01118, c = 091;
    printf("%d %d %d\n", a, b, c); 
    return 0;
}
EOF

tests/driver.sh Show resolved Hide resolved
Add a test case with invalid octal numbers to 'tests/driver.sh'
to ensure the lexer/parser can detect invalid octal numbers.
@jserv jserv merged commit 3573b9f into sysprog21:master Sep 16, 2024
4 checks passed
@jserv
Copy link
Collaborator

jserv commented Sep 16, 2024

Thank @DrXiao for contributing!

@DrXiao DrXiao deleted the fix-parsing-octal branch September 16, 2024 02:36
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.

4 participants