CS50 recover程序Valgrind检测出Segmentation Fault问题排查求助

问题背景 #

完成CS50的recover任务后，自行运行程序无任何错误，但使用Valgrind检测时返回Segmentation Fault (core dumped)错误。已做内存权限检查、关闭文件后置NULL、释放所有指针等操作，仍无法解决。

程序代码 #

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <string.h>

int main(int argc, char *argv[])
{
    if (argc != 2) {
        printf("Usage: ./recover [argument]\n");
        return 1;
    }

    FILE* sdCard = fopen(argv[1], "r");

    if (sdCard == NULL) {
        printf("Couldn't open %s in read-mode", argv[1]);
        return 1;
    }

    uint8_t buffer[512];

    // ###.png\0 => 8 characters
    char file_name[8];

    int fileNumber = 0;

    FILE* recoveredJPG;
    char* zeroes = NULL;

    int foundFirstJPEG = 0;

    // Create files
    while(fread(buffer, sizeof(uint8_t), 512, sdCard)) {
        if (buffer[0] == 0xFF && buffer[1] == 0xD8 && buffer[2] == 0xFF && (buffer[3] & 0xFF) == 0xE0) {
            foundFirstJPEG = 1;

            if (fileNumber < 10) {
                zeroes = malloc(3*sizeof(char));

                if (zeroes == NULL) {
                    printf("Memory allocation failed!");
                    fclose(sdCard);
                    sdCard = NULL;
                    if (recoveredJPG != NULL) {
                        fclose(recoveredJPG);
                        recoveredJPG = NULL;
                    }
                    return 1;
                }

                strcpy(zeroes, "00");
            } else if (fileNumber >= 10) {
                zeroes = malloc(2*sizeof(char));

                if (zeroes == NULL) {
                    printf("Memory allocation failed!");
                    fclose(sdCard);
                    sdCard = NULL;
                    if (recoveredJPG != NULL) {
                        fclose(recoveredJPG);
                        recoveredJPG = NULL;
                    }
                    return 1;
                }

                strcpy(zeroes, "0");
            }

            if (zeroes == NULL) {
                printf("Memory allocation failed!");
                fclose(sdCard);
                sdCard = NULL;
                if (recoveredJPG != NULL) {
                    fclose(recoveredJPG);
                    recoveredJPG = NULL;
                }
                return 1;
            }

            sprintf(file_name, "%s%i.jpg", zeroes, fileNumber);
            if (recoveredJPG != NULL){
                fclose(recoveredJPG);
                recoveredJPG = NULL;
            }
            recoveredJPG = fopen(file_name, "w");
            if (recoveredJPG == NULL) {
                printf("Error while creating %s.", file_name);
                free(zeroes);
                zeroes = NULL;
                fclose(sdCard);
                sdCard = NULL;
                return 1;
            }
            free(zeroes);
            zeroes = NULL;
            fwrite(buffer, sizeof(uint8_t), 512, recoveredJPG);
            fileNumber++;
        } else if (foundFirstJPEG) {
            fwrite(buffer, sizeof(uint8_t), 512, recoveredJPG);
        }
    }
    if (recoveredJPG != NULL) {
        fclose(recoveredJPG);
        recoveredJPG = NULL;
    }
    fclose(sdCard);
    sdCard = NULL;


    if (zeroes != NULL) {
        free(zeroes);
        zeroes = NULL;
    }

    return 0;
}

Valgrind检测输出 #

==3125== Memcheck, a memory error detector
==3125== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==3125== Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info
==3125== Command: ./recover card.raw
==3125== 
==3125== Conditional jump or move depends on uninitialised value(s)
==3125==    at 0x10948C: main (recover.c:81)
==3125==  Uninitialised value was created by a stack allocation
==3125==    at 0x1091C4: main (recover.c:7)
==3125== 
==3125== Use of uninitialised value of size 8
==3125==    at 0x4A0E294: fclose@@GLIBC_2.2.5 (iofclose.c:48)
==3125==    by 0x10949D: main (recover.c:82)
==3125==  Uninitialised value was created by a stack allocation
==3125==    at 0x1091C4: main (recover.c:7)
==3125== 
==3125== Use of uninitialised value of size 8
==3125==    at 0x4A0E33C: fclose@@GLIBC_2.2.5 (iofclose.c:51)
==3125==    by 0x10949D: main (recover.c:82)
==3125==  Uninitialised value was created by a stack allocation
==3125==    at 0x1091C4: main (recover.c:7)
==3125== 
==3125== Invalid read of size 8
==3125==    at 0x4A0E353: fclose@@GLIBC_2.2.5 (iofclose.c:51)
==3125==    by 0x10949D: main (recover.c:82)
==3125==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
==3125== 
==3125== 
==3125== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==3125==  Access not within mapped region at address 0x8
==3125==    at 0x4A0E353: fclose@@GLIBC_2.2.5 (iofclose.c:51)
==3125==    by 0x10949D: main (recover.c:82)
==3125==  If you believe this happened as a result of a stack
==3125==  overflow in your program's main thread (unlikely but
==3125==  possible), you can try to increase the size of the
==3125==  main thread stack using the --main-stacksize= flag.
==3125==  The main thread stack size used in this run was 8388608.
==3125== 
==3125== HEAP SUMMARY:
==3125==     in use at exit: 4,571 bytes in 3 blocks
==3125==   total heap usage: 3 allocs, 0 frees, 4,571 bytes allocated
==3125== 
==3125== 3 bytes in 1 blocks are still reachable in loss record 1 of 3
==3125==    at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==3125==    by 0x1092F2: main (recover.c:38)
==3125== 
==3125== 472 bytes in 1 blocks are still reachable in loss record 2 of 3
==3125==    at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==3125==    by 0x4A0EE6E: __fopen_internal (iofopen.c:65)
==3125==    by 0x4A0EE6E: fopen@@GLIBC_2.2.5 (iofopen.c:86)
==3125==    by 0x109210: main (recover.c:13)
==3125== 
==3125== 4,096 bytes in 1 blocks are still reachable in loss record 3 of 3
==3125==    at 0x4846828: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==3125==    by 0x4A0E1A4: _IO_file_doallocate (filedoalloc.c:101)
==3125==    by 0x4A1E513: _IO_doallocbuf (genops.c:347)
==3125==    by 0x4A1CF3B: _IO_file_xsgetn (fileops.c:1288)
==3125==    by 0x4A0F48A: fread (iofread.c:38)
==3125==    by 0x10927A: main (recover.c:33)
==3125== 
==3125== LEAK SUMMARY:
==3125==    definitely lost: 0 bytes in 0 blocks
==3125==    indirectly lost: 0 bytes in 0 blocks
==3125==      possibly lost: 0 bytes in 0 blocks
==3125==    still reachable: 4,571 bytes in 3 blocks
==3125==         suppressed: 0 bytes in 0 blocks
==3125== 
==3125== For lists of detected and suppressed errors, rerun with: -s
==3125== ERROR SUMMARY: 4 errors from 4 contexts (suppressed: 0 from 0)
/opt/cs50/bin/valgrind: line 13:  3125 Segmentation fault      (core dumped) /usr/bin/valgrind "$@"

错误原因 #

从Valgrind的报错信息可以明确定位问题：

1. 未初始化指针导致非法操作：代码中FILE* recoveredJPG;定义时未初始化为NULL，栈上的这个指针会保留随机的垃圾值。
2. 条件判断依赖未初始化值：程序末尾的if (recoveredJPG != NULL)判断会使用这个垃圾值，当垃圾值恰好不是NULL时，会执行fclose(recoveredJPG);。
3. 非法内存访问触发段错误：此时recoveredJPG指向的是无效内存（如报错中的地址0x8），调用fclose会直接触发Segmentation Fault。

只需将FILE* recoveredJPG;修改为FILE* recoveredJPG = NULL;，确保指针初始状态有效，即可解决该问题。

内容的提问来源于stack exchange，提问作者NeeRaX