grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] Bug fix for loadenv.c


From: Bean
Subject: [PATCH] Bug fix for loadenv.c
Date: Fri, 4 Sep 2009 03:20:30 +0800

Hi,

This is the bug fix for command/loadenv.c. Here is a small program to
illustrate what happen:

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

struct data
{
  struct data *next;
  int length;
};

struct data *
new_data ()
{
  struct data *p;

  p = malloc (sizeof (*p));
  p->length = 1;
  p->next = 0;

  return p;
}

int
main ()
{
  int index;
  struct data *head, *p;

  head = new_data ();
  for (p = head, index = 0; p; p = p->next, index += p->length)
    printf ("%d\n", index);

  return 0;
}

Compile it with gcc-4.0 from OSX:
i686-apple-dawin8-gcc-4.0.1 -Os test.c

The assembly dump shows:

L4:
        movl    %edi, 4(%esp)
        movl    -28(%ebp), %eax
        movl    %eax, (%esp)
        call    L_printf$stub
        movl    (%esi), %esi
        addl    4(%esi), %edi
        jmp     L4

Why would it uses unconditional jumps ? I think it's some over
optimization in gcc. We have index += p->length after p = p->next, if
p is null, p->length would be invalid, so gcc figure that p would
never be null and the end condition is always false. In fact, the bug
is in the code itself, even if it tests p, in OS environment, access
null pointer is not allowed, so index += p->length can't be executed
anyway.

The correct way is:

  for (p = head, index = 0; p;)
  {
    printf ("%d\n", index);
   p = p->next;
  if (p)
   index += p->length;
  }


-- 
Bean

gitgrub home: http://github.com/grub/grub/
my fork page: http://github.com/bean123/grub/

Attachment: loadenv.diff
Description: Binary data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]