Erreur de segmentation avec strcat

Question

J'ai un petit problème avec les fautes strcat et de segmentation. L'erreur est la suivante:

Program received signal EXC_BAD_ACCESS, Could not access memory. 
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000 
0x00007fff82049f1f in __strcat_chk() 
(gdb) where 
#0 0x00007fff82049f1f in __strcat_chk() 
#1 0x0000000100000adf in bloom_operation (bloom=0x100100080, item=0x100000e11 "hello world", operation=1) at bloom_filter.c:81 
#2 0x0000000100000c0e in bloom_insert (bloom=0x100100080, to_insert=0x100000e11 "hello world") at bloom_filter.c:99 
#3 0x0000000100000ce5 in main() at test.c:6 

bloom_operation est la suivante:

int bloom_operation(bloom_filter_t *bloom, const char *item, int operation) 
{ 
    int i;  

    for(i = 0; i < bloom->number_of_hash_salts; i++) 
    { 
     char temp[sizeof(item) + sizeof(bloom->hash_salts[i]) + 2]; 
     strcat(temp, item); 
     strcat(temp, *bloom->hash_salts[i]); 

     switch(operation) 
     { 
      case BLOOM_INSERT: 
       bloom->data[hash(temp) % bloom->buckets] = 1; 
       break; 
      case BLOOM_EXISTS: 
       if(!bloom->data[hash(temp) % bloom->buckets]) return 0; 
       break; 
     }  
    } 

    return 1; 
} 

La ligne de trouble est le secondes strcat. Le bloom-> hash_salts font partie d'une structure définie comme suit:

typedef unsigned const char *hash_function_salt[33]; 
typedef struct { 
    size_t buckets;  
    size_t number_of_hash_salts; 
    int bytes_per_bucket; 
    unsigned char *data; 
    hash_function_salt *hash_salts; 
} bloom_filter_t; 

Et ils sont initialisés ici:

bloom_filter_t* bloom_filter_create(size_t buckets, size_t number_of_hash_salts, ...) 
{ 
    bloom_filter_t *bloom; 
    va_list args; 
    int i; 

    bloom = malloc(sizeof(bloom_filter_t)); 
    if(bloom == NULL) return NULL; 

    // left out stuff here for brevity... 

    bloom->hash_salts = calloc(bloom->number_of_hash_salts, sizeof(hash_function_salt)); 

    va_start(args, number_of_hash_salts); 

    for(i = 0; i < number_of_hash_salts; ++i) 
     bloom->hash_salts[i] = va_arg(args, hash_function_salt); 

    va_end(args); 

    // and here... 
} 

Et bloom_filter_create est appelé comme suit:

bloom_filter_create(100, 4, "3301cd0e145c34280951594b05a7f899", "0e7b1b108b3290906660cbcd0a3b3880", "8ad8664f1bb5d88711fd53471839d041", "7af95d27363c1b3bc8c4ccc5fcd20f32"); 

Je Je fais quelque chose de mal mais je suis vraiment perdu sur quoi. Merci d'avance,

Ben.

Answer 1

Vous devez utiliser strlen, pas sizeof. item est passé en tant que pointeur, pas un tableau.

La ligne:

char temp[sizeof(item) + sizeof(bloom->hash_salts[i]) + 2]; 

fera la température 34x la longueur d'un pointeur + 2. La taille de l'élément est la taille d'un pointeur et le sizeof(bloom->hash_salts[i]) est actuellement 33x la taille d'un pointeur.

Vous devez utiliser strlen pour item, afin de connaître le nombre réel de caractères.

Ensuite, bloom->hash_salts[i] est un hash_function_salt, qui est un tableau de 33 pointeurs vers char. Il semble que hash_function_salt devrait être défini comme:

puisque vous voulez qu'il puisse contenir 33 caractères, pas 33 pointeurs. Vous devez également vous rappeler que lorsque vous passez un littéral de chaîne à bloom_filter_create, vous passez un pointeur. Cela signifie que pour initialiser le tableau hash_function_salt, nous utilisons memcpy ou strcpy. memcpy est plus rapide quand on sait la longueur exacte (comme ici):

Nous obtenons donc:

typedef unsigned char hash_function_salt[33]; 

et bloom_filter_create:

memcpy(bloom->hash_salts[i], va_arg(args, char*), sizeof(bloom->hash_salts[i])); 

revenir à bloom_operation, nous obtenons:

char temp[strlen(item) + sizeof(bloom->hash_salts[i])]; 
strcpy(temp, item); 
strcat(temp, bloom->hash_salts[i]); 

Nous utilisons strlen pour l'élément puisque c'est un pointeur, mais sizeof pour le hash_function_salt, qui est un tableau de taille fixe de char. Nous n'avons pas besoin d'ajouter quoi que ce soit, car hash_function_salt inclut déjà de la place pour un NUL. Nous utilisons d'abord strcpy. strcat est pour quand vous avez déjà une chaîne terminée par NUL (que nous ne sommes pas ici). Notez que nous laissons tomber le *. C'était une erreur suite à votre typedef incorrect.

Answer 2

Je vois quelques problèmes:

char temp[sizeof(item) + sizeof(bloom->hash_salts[i]) + 2]; 

Le sizeof(item) ne retournera 4 (ou 8 sur une plate-forme 64 bits). Vous devez probablement utiliser strlen() pour la longueur réelle. Bien que je ne pense pas que vous pouvez le déclarer sur la pile comme ça avec strlen (bien que je pense peut-être que j'ai vu quelqu'un indiquer que c'était possible avec les nouvelles versions de gcc - je suis peut-être sur le coup).

L'autre problème est que le tableau temporaire n'est pas initialisé. Ainsi, le premier strcat peut ne pas écrire au début du tableau. Il doit avoir une valeur NULL (0) dans le premier élément avant d'appeler strcat.

Il est peut-être déjà dans le code qui a été supprimé, mais je n'ai pas vu que vous avez initialisé le membre number_of_hash_salts dans la structure.

Answer 3

Votre calcul de la taille de tableau pour temp utilise sizeof(bloom->hash_salts[i]) (qui est juste la taille du pointeur ), mais vous déréférencer le pointeur et essayez de copier toute la chaîne en temp.

Answer 4

Etes-vous sûr que le type hash_function_salt est défini correctement? Vous pouvez avoir un trop grand nombre * s:

(gdb) ptype bloom 
type = struct { 
    size_t buckets; 
    size_t number_of_hash_salts; 
    int bytes_per_bucket; 
    unsigned char *data; 
    hash_function_salt *hash_salts; 
} * 
(gdb) ptype bloom->hash_salts 
type = const unsigned char **)[33] 
(gdb) ptype bloom->hash_salts[0] 
type = const unsigned char *[33] 
(gdb) ptype *bloom->hash_salts[0] 
type = const unsigned char * 
(gdb) 

Answer 5

D'abord, comme tout le monde a dit, vous avez la taille temp en fonction des tailles de deux pointeurs, et non les longueurs des cordes. Vous l'avez corrigé et signalez que le symptôme a été déplacé vers l'appel strlen().

Ceci montre un bug plus subtil. Vous avez initialisé le tableau bloom->hash_salts[] à partir des pointeurs renvoyés par va_arg(). Ces pointeurs auront une durée de vie limitée. Ils ne peuvent même pas survivre à l'appel à va_end(), mais ils ne presque certainement pas survivre à l'appel à bloom_filter_create(). Plus tard, dans bloom_filter_operation(), ils pointent vers des endroits arbitraires et vous êtes condamné à une sorte d'échec intéressant.

Édition: Pour résoudre ce problème, les pointeurs stockés dans le groupe hash_salts doivent avoir une durée de vie suffisante. Une façon de traiter qui est d'allouer de la mémoire pour eux, de les copier sur le tableau de varargs, par exemple:

// fragment from bloom_filter_create() 
bloom->hash_salts = calloc(bloom->number_of_hash_salts, sizeof(hash_function_salt)); 
va_start(args, number_of_hash_salts); 
for(i = 0; i < number_of_hash_salts; ++i) 
     bloom->hash_salts[i] = strdup(va_arg(args, hash_function_salt)); 
va_end(args); 

plus tard, vous devez faire une boucle sur hash_salts et appeler free() sur chaque élément avant de libérer le tableau des pointeurs lui-même.

Une autre approche qui nécessiterait plus de temps système pour initialiser, mais moins d'effort pour libérer serait d'allouer le tableau de pointeurs avec suffisamment d'espace pour toutes les chaînes dans une seule allocation. Copiez ensuite les chaînes et remplissez les pointeurs. C'est beaucoup de code pour obtenir un très petit avantage.