[SOLVED] SEGMENTATION FAULT using gcc 4.4.4 -O2 , works with gcc 4.1.0 -O2 or gcc 4.4.4 -O1

amir1981 · 07-26-2010, 02:51 PM

Quote:

Originally Posted by johnsfine

OK, now I see I misunderstood GDB output regarding 818 vs. 828. That is just a difference in the optimizer behavior of the two compiles.

I don't know how much to trust GDB regarding the values of this and value_d when stopped at line 818. Generally I don't trust any implausible variable values reported by GDB. GDB and/or the compiler are not very good at tracking which variables are in which registers and/or stack locations at which lines of the source code.

zirias expressed the opinion (that I mostly share) that 0x0x7f00000000 is an unreasonable value for this. You told me that value_d is a member of SysString so at line 818 value_d should be equivalent to this->value_d which (assuming this is invalid) should have been Cannot access memory at address rather than $4 = (unichar *) 0x0

If I were debugging it, I would poke around a bit more at that point to find out which, if any, of the apparently contradictory pieces of info represent the result of the bug you're looking for, vs. which represent wrong info displayed by GDB.

At 818 and maybe at an s further into that function, I would want to know what is:
this
&value_d
this->value_d
If those don't start to add up to something consistent, I'd look at disassembly of the code at that point and at register values and also try directly looking at memory at address 0x7f00000000

Breakpoint 1, SysString::diagnose (level_a=<value optimized out>) at sstr_02.cc:221
221 num.assign(dbyte, L"asdf = %u xyz");
Current language: auto; currently c++
(gdb) p &num
$10 = (SysString *) 0x7fbfffe2a0

(gdb) p num.value_d
$1 = (unichar *) 0x61fcb0
(gdb) s
SysString::assign (this=0x7f00000000, arg_a=27 '\033', fmt_a=0x41afd8) at sstr_03.cc:818
818 bool8 SysString::assign(byte8 arg_a, const unichar* fmt_a) {
(gdb) p this
$2 = (SysString * const) 0x7f00000000
(gdb) p &value_d
$3 = (unichar **) 0x7f00000000
(gdb) p this->value_d
$4 = (unichar *) 0x0
(gdb) s
828 if (fmt_a == (unichar*)NULL) {
(gdb) p this
$5 = (SysString * const) 0x7f00000000
(gdb) p &value_d
$6 = (unichar **) 0x7f00000000
(gdb) p this->value_d
$7 = (unichar *) 0x0
(gdb)

Now what? Should not &num and this point to the same memory?

johnsfine · 07-26-2010, 02:58 PM

That's a bit of a surprise. 0x7f00000000 seems to be a valid address.

So that makes the original seg fault less plausible.

Your post #9 makes it look like there was a seg fault at

Code:

SysString::clear (this=0x7f00000000, cmode_a=Integral::RESET) at sstr_03.cc:1623
1623 if (capacity_d > 0) {

Is GDB wrong about the line number for that seg fault, or did 0x7f00000000 stop being a valid address by the time the code reached that point?

The latter should be easy to determine by just setting a breakpoint there and proceeding to it and seeing what this and capacity_d and &capacity_d all are.

Assuming GDB might be wrong about the line number of the seg fault, we'd also like to know what value_d is at that point.

Edit: Sorry, I wasn't thinking clearly. We already know value_d was bad before it reached there, so we can reasonably assume GDB is wrong about the line number of the seg fault and you need to look earlier not later to find out when/why value_d was clobbered.

amir1981 · 07-26-2010, 03:05 PM

Quote:

Originally Posted by johnsfine

That's a bit of a surprise. 0x7f00000000 seems to be a valid address.

So that makes the original seg fault less plausible.

Your post #9 makes it look like there was a seg fault at

Code:

SysString::clear (this=0x7f00000000, cmode_a=Integral::RESET) at sstr_03.cc:1623
1623 if (capacity_d > 0) {

Is GDB wrong about the line number for that seg fault, or did 0x7f00000000 stop being a valid address by the time the code reached that point?

The latter should be easy to determine by just setting a breakpoint there and proceeding to it and seeing what this and capacity_d and &capacity_d all are.

I don't know why, but seems suddenly the pointer to "num" object changes from its value to 0x7f00000000, because before calling "assign"
the address of num is 0x7fbfffe2a0 and just after calling it changes
What kind of things could do this?

johnsfine · 07-26-2010, 03:08 PM

Quote:

Originally Posted by amir1981

Breakpoint 1, SysString::diagnose (level_a=<value optimized out>) at sstr_02.cc:221
(gdb) p num.value_d
$3 = (unichar *) 0x61fcb0
(gdb) s
SysString::assign (this=0x7f00000000, arg_a=27 '\033', fmt_a=0x41afd8) at sstr_03.cc:818
(gdb) p value_d
$4 = (unichar *) 0x0

I discounted the significance of the above earlier, because I didn't trust gdb.

Now, I assume you showed us that because you think num at line sstr_02.cc:221 is the same object as *this at line sstr_03.cc:818. So you should validate that by having gdb give the vale of &num at line sstr_02.cc:221

Edit: you answered that while I was asking the question.

johnsfine · 07-26-2010, 03:12 PM

Quote:

Originally Posted by johnsfine

post the area around each of those lines (around 221 in sstr_02.cc

That seems to be the approximate location of the problem and you seem to have ignored my earlier request to post that code.

Quote:

Originally Posted by amir1981

seems suddenly the pointer to "num" object changes from its value to 0x7f00000000, because before calling "assign"
the address of num is 0x7fbfffe2a0 and just after calling it changes
What kind of things could do this?

Notice the low 32 bits of the pointer became zero.

That is exactly the kind of bug typical of an error in porting 32 bit code to 64 bit.

This could easily be caused by the immediately preceding object in memory being stored as 64 bits into a 32 bit allocated space overwriting the next 32 bits with zero.

Note I mean the object preceding the pointer to num, not the object preceding num itself.

I could be a lot more specific if I saw all the code from the declaration of num through line 221.

amir1981 · 07-26-2010, 03:14 PM

Quote:

Originally Posted by johnsfine

I discounted the significance of the above earlier, because I didn't trust gdb.

Now, I assume you showed us that because you think num at line sstr_02.cc:221 is the same object as *this at line sstr_03.cc:818. So you should validate that by having gdb give the vale of &num at line sstr_02.cc:221

Edit: you answered that while I was asking the question.

on 32-bit machine:

Breakpoint 1, SysString::diagnose (level_a=Integral::BRIEF) at sstr_02.cc:221
221 num.assign(dbyte, L"asdf = %u xyz");
(gdb) p &num
$3 = (SysString *) 0xbfffebc4
(gdb) s
SysString::assign (this=0xbfffebc4, arg_a=27 '\033',
fmt_a=0x8063ac0 L"asdf = %u xyz") at sstr_03.cc:828
828 if (fmt_a == (unichar*)NULL) {
(gdb) p this
$4 = (SysString * const) 0xbfffebc4
(gdb)

so &num and this are the same

on 64bit:

Breakpoint 1, SysString::diagnose (level_a=<value optimized out>) at sstr_02.cc:221
221 num.assign(dbyte, L"asdf = %u xyz");
(gdb) p &num
$15 = (SysString *) 0x7fbfffe2a0
(gdb) s
SysString::assign (this=0x7f00000000, arg_a=27 '\033', fmt_a=0x41afd8) at sstr_03.cc:818
818 bool8 SysString::assign(byte8 arg_a, const unichar* fmt_a) {
(gdb) p this
$16 = (SysString * const) 0x7f00000000
(gdb)

This shows that the address for num has changed , right?

johnsfine · 07-26-2010, 03:20 PM

Quote:

Originally Posted by amir1981

This shows that the address for num has changed , right?

Right. I already concluded that in posts above. Reread above to see what you missed while you were constructing that post.

amir1981 · 07-26-2010, 03:21 PM

Quote:

Originally Posted by johnsfine

Notice the low 32 bits of the pointer became zero.

That is exactly the kind of bug typical of an error in porting 32 bit code to 64 bit.

This could easily be caused by the immediately preceding object in memory being stored as 64 bits into a 32 bit allocated space overwriting the next 32 bits with zero.

Note I mean the object preceding the pointer to num, not the object preceding num itself.

I could be a lot more specific if I saw all the code from the declaration of num through line 221

this is that part of the code:

Code:

// make sure that an empty string fails
  //
  SysString num;
  int32 i;
  if (num.get(i)) {
    return Error::handle(name(), L"get", Error::TEST, __FILE__, __LINE__);
  }

  // make sure that non numeric types fail for get
  //
  num.assign(L"abc");
  if (num.get(i)) {
    return Error::handle(name(), L"get", Error::TEST, __FILE__, __LINE__);
  }

  // make sure that strings greater than length 1 fail for a get on SysChar
  //
  SysChar tmp_char;
  if (num.get(tmp_char)) {
    return Error::handle(name(), L"get", Error::TEST, __FILE__, __LINE__);
  }

  // setup temporary variables
  //
  byte8 dbyte = (int32)27;
  byte8 dbyte_v;

  int16 dshort = (int32)27;
  int16 dshort_v;

  int32 dlong = (int32)277;
  int32 dlong_v;
  
  int64 dllong = (int64)13020756033LL;
  int64 dllong_v;
  
  uint16 dushort = (uint16)6907;
  uint16 dushort_v;
  
  uint32 dulong = (uint32)2777;
  uint32 dulong_v;
  
  uint64 dullong = (uint64)1302075603332LL;
  uint64 dullong_v;

  float32 dfloat = (float32)27.27e-19;
  float32 dfloat_v;
  
  float64 ddouble = (float64)272727272727.272727e+20;
  float64 ddouble_v;

  bool8 dboolean = true;
  bool8 dboolean_v;
  
  void* dvoidp = (void*)27;
  void* dvoidp_v;

  // test the byte conversions
  //
  num.assign(dbyte);
  num.get(dbyte_v);

  if (dbyte != dbyte_v) {
    Error::handle(name(), L"assign(byte8)", Error::TEST, __FILE__, __LINE__);
  }

  num.assign(dbyte, L"asdf = %u xyz");

johnsfine · 07-26-2010, 03:24 PM

That makes it look like the call to num.get(dbyte_v); did the harm.

Is the source code to get posted yet?

num is a local variable in the current stack frame. GDB deduces its address (at line 221) from the rbp register. That register must not be corrupted or gdb would be totally confused at that point.

The only way to get this symptom is if the optimizer had put the address of num into a callee saved register (because it is used so much) rather than recompute it from ebp each time. That's strange because recomputing it from ebp is nearly free compared to simply copying it into edi (all that would make sense if you knew x86_64 asm).

amir1981 · 07-26-2010, 03:33 PM

Quote:

Originally Posted by johnsfine

That makes it look like the call to num.get(dbyte_v); did the harm.

Is the source code to get posted yet?

Here is the code for "get"

Code:

// method: get
//
// arguments:
//  void*& val: (output) pointer value
//
// return: a bool8 value indicating status
//
// this method converts the object into an address pointer
//
bool8 SysString::get(void*& val_a) const {

  // declare a null pointer
  //
  val_a = (void*)NULL;

  // check if string is null string
  //
  if (firstStr((unichar*)NULL_PTR) >= 0) {
    return true;
  }
    
  // use the 8-bit character conversion
  //
  if (sscanf((char*)(byte8*)(*this),
	     (char*)DEF_FMT_VOIDP_8BIT, &val_a) != 1) {
    return false;
  }

  // exit gracefully
  //
  return true;
}

// method: get
//
// arguments:
//  bool8& val: (output) bool8 value
//
// return: a bool8 value indicating status
//
// this method converts the string into a bool8 value
//
bool8 SysString::get(bool8& val_a) const {

  // initialize the return value
  //
  bool8 status = false;
  
  // return bool8 true or false
  //
  if (eq((unichar*)BOOL_TRUE)) {
    val_a = true;
    status = true;
  }
  else if (eq((unichar*)BOOL_FALSE)) {
    val_a = false;
    status = true;
  }

  // exit gracefully
  //
  return status;
}

// method: get
//
// arguments:
//  byte8& val: (output) byte value
//
// return: a bool8 value indicating status
//
// this method converts the object into a ubyte8 integer
//
bool8 SysString::get(byte8& val_a) const {

  // read in the integer
  //
  uint32 val = 0;

  // use the 8-bit character conversion
  //
  if (sscanf((char*)(byte8*)(*this),
	     (char*)DEF_FMT_LONG_8BIT, &val) != 1) {
    return false;
  }

  // assign it to a byte8
  //
  val_a = (byte8)val;
  
  // exit gracefully
  //
  return true;
}

// method: get
//
// arguments:
//  unichar& val: (output) unichar value
//
// return: a bool8 value indicating status
//
// this method converts the string into a unichar value
//
bool8 SysString::get(unichar& val_a) const {

  // if length is 1 then assign zeroth element to unichar
  //
  if (length() == 1) {
    val_a = value_d[0];
    return true;
  }

  // exit ungracefully
  //
  return false;
}

// method: get
//
// arguments:
//  uint16& val: (output) uint16 value
//
// return: a bool8 value indicating status
//
// this method converts the object into a uint16 integer
//
bool8 SysString::get(uint16& val_a) const {

  // declare local variables
  //
  uint32 tmp_val = 0;

  // use the 8-bit character conversion
  //
  if (sscanf((char*)(byte8*)(*this),
	     (char*)DEF_FMT_ULONG_8BIT, &tmp_val) != 1) {
    return false;
  }

  // set the output
  //
  val_a = tmp_val;

  // exit gracefully
  //
  return true;
}

// method: get
//
// arguments:
//  uint32& val: (output) uint32 value
//
// return: a bool8 value indicating status
//
// this method converts the object into a uint32 integer
//
bool8 SysString::get(uint32& val_a) const {

  // declare local variables
  //
  val_a = 0;

  // use the 8-bit character conversion
  //
  if (sscanf((char*)(byte8*)(*this),
	     (char*)DEF_FMT_ULONG_8BIT, &val_a) != 1) {
    return false;
  }

  // exit gracefully
  //
  return true;
}

// method: get
//
// arguments:
//  uint64& val: (output) uint64 value
//
// return: a bool8 value indicating status
//
// this method converts the object into a uint64 integer
//
bool8 SysString::get(uint64& val_a) const {

  // declare local variables
  //
  val_a = 0;

  // use the 8-bit character conversion
  //
  if (sscanf((char*)(byte8*)(*this),
	     (char*)DEF_FMT_ULLONG_8BIT, &val_a) != 1) {
    return false;
  }

  // exit gracefully
  //
  return true;
}

// method: get
//
// arguments:
//  int16& val: (output) int16 value
//
// return: a bool8 value indicating status
//
// this method converts the object into a int16 integer
//
bool8 SysString::get(int16& val_a) const {

  // declare local variable
  //
  int32 tmp_val = 0;

  // use the 8-bit character conversion
  //
  if (sscanf((char*)(byte8*)(*this),
	     (char*)DEF_FMT_LONG_8BIT, &tmp_val) != 1) {
    return false;
  }

  // set the output
  //
  val_a = tmp_val;
  
  // exit gracefully
  //
  return true;
}

// method: get
//
// arguments:
//  int32& val: (output) int32 value
//
// return: a bool8 value indicating status
//
// this method converts the object into a int32 integer
//
bool8 SysString::get(int32& val_a) const {

  // declare local variables
  //
  val_a = 0;

  // use the 8-bit character string conversion
  //
  if (sscanf((char*)(byte8*)(*this),
	     (char*)DEF_FMT_LONG_8BIT, &val_a) != 1) {
    return false;
  }

  // exit gracefully
  //
  return true;
}

// method: get
//
// arguments:
//  int64& val: (output) int64 value
//
// return: a bool8 value indicating status
//
// this method converts the object into a int64 integer
//
bool8 SysString::get(int64& val_a) const {

  // declare local variables
  //
  val_a = 0;

  // use the 8-bit character conversion
  //
  if (sscanf((char*)(byte8*)(*this),
	     (char*)DEF_FMT_LLONG_8BIT, &val_a) != 1) {
    return false;
  }

  // exit gracefully
  //
  return true;
}

// method: get
//
// arguments:
//  float32& val: (output) float32 value
//
// return: a bool8 value indicating status
//
// this method converts the object to a single precision floating point number
//
bool8 SysString::get(float32& val_a) const {

  // declare local variables
  //
  val_a = (float32)0.0;

  // use the 8-bit character conversion
  //
  if (sscanf((char*)(byte8*)(*this),
	     (char*)DEF_RFMT_FLOAT_8BIT, &val_a) != 1) {
    return false;
  }

  // exit gracefully
  //
  return true;
}

// method: get
//
// arguments:
//  float64& val: (output) float64 value
//
// return: a bool8 value indicating status
//
// this method converts the object to a float64 precision floating point number
//
bool8 SysString::get(float64& val_a) const {

  // declare local variables
  //
  val_a = (float64)0.0;

  // use the 8-bit character conversion
  //
  if (sscanf((char*)(byte8*)(*this),
	     (char*)DEF_RFMT_DOUBLE_8BIT, &val_a) != 1) {
    return false;
  }

  // exit gracefully
  //
  return true;
}

// method: get
//
// arguments:
//  SysComplex<TIntegral>& arg: (output) complex value
//
// return: a bool8 value indicating status
//
// this method converts the object to a complex number
// 
template <class TIntegral>
bool8 SysString::get(SysComplex<TIntegral>& arg_a) const {

  // declare local variable
  //
  SysString str(*this);
  str.trim();
  int32 imag_pos = str.firstChr(L'j');
  if ((imag_pos > 0) && (imag_pos != str.length() - 1)) {
    return Error::handle(name(), L"get", Error::ARG, __FILE__, __LINE__);
  }

  // if there is not 'j' in the string, convert directly
  //
  if (imag_pos < 0) {
    TIntegral val;
    str.get(val);
    arg_a = SysComplex<TIntegral>(val, 0);

    int32 pos = 0;
    int32 len = str.length();
    SysString num;

    // search letters '+' or '-' in the string
    //
    str.tokenize(num, pos, L'+');
    if (pos >= len - 1) {
      pos = 0;
      str.tokenize(num, pos, L'-');
    }

    // if '+' or '-' exists in the string
    //
    if (pos < len - 1 ) {
      str.debug(L"value");
      return Error::handle(name(), L"invalid format - complex numbers should be in the format: a+bj", Error::ARG, __FILE__, __LINE__);
    }
  }

  else {

    // declare local variable
    //
    int32 pos = 0;
    int32 len = str.length();
    bool8 isPositive = true;
    TIntegral val0, val1;
    SysString num;

    // search letters '+' or '-' in the string
    //
    str.tokenize(num, pos, L'+');
    if (pos >= len - 1) {
      pos = 0;
      str.tokenize(num, pos, L'-');
      isPositive = false;
    }

    // if '+' or '-' exists in the string
    //
    if (pos < len - 1 ) {

      // get the real part of the complex number
      //
      num.trim();
      num.get(val0);

      // set the appropriate sign for real part if necessary
      //
      if ((str(0) != num(0)) && (str(0) == '-')) {
	val0 = -val0;
      }

      // get the image part of the complex number
      //
      pos++;
      str.tokenize(num, pos, L'j');
      num.trim();
      if (num.length() == 0) {
	val1 = 1;
      }
      else {
	if (!num.get(val1)) {
	  str.debug(L"value");
	  return Error::handle(name(), L"invalid format - complex numbers should be in the format: a+bj", Error::ARG, __FILE__, __LINE__);
	}
      }

      // set the corresponding sign for the image part
      //
      if (!isPositive) {
	val1 = -val1;
      }

      // copy temporary complex number to output argument
      //
      arg_a = SysComplex<TIntegral>(val0, val1);
    }

    // only real part or image part exists in the string
    //
    else {
      
      // delete the letter 'j'
      //
      str.deleteRange(imag_pos, 1);

      // get the image part of the complex number
      //
      TIntegral val;
      str.trim();
      if (str.length() == 0 || str.eq(L"+")) {
	val = 1;
      }
      else if (str.eq(L"-")) {
	val = -1;
      }
      else {
	if (!str.get(val)) {
	  str.debug(L"value");
	  return Error::handle(name(), L"invalid format - complex numbers should be in the format: a+bj", Error::ARG, __FILE__, __LINE__);
	}
      }
      
      // copy temporary complex number to output argument
      //
      arg_a = SysComplex<TIntegral>(0, val);
    }
  }

  // exit gracefully
  //
  return true;
}
  
// explicit instantiations for complex types
//
template
bool8 SysString::get<float32>(SysComplex<float32>&) const;

template
bool8 SysString::get<float64>(SysComplex<float64>&) const;

template
bool8 SysString::get<int32>(SysComplex<int32>&) const;

// method: get
//
// arguments:
//  SysChar& val: (output) SysChar value
//
// return: a bool8 value indicating status
//
// this method converts the object into a SysChar
//
bool8 SysString::get(SysChar& val_a) const {

  // if length is 1 then assign the element to output SysChar
  //
  if (length() == 1) {
    return val_a.assign(value_d[0]);
  }

  // exit ungracefully
  //
  return false;
}

johnsfine · 07-26-2010, 03:39 PM

I'm pretty sure this is the bug:

Code:

  uint32 val = 0;

  // use the 8-bit character conversion
  //
  if (sscanf((char*)(byte8*)(*this),
	     (char*)DEF_FMT_LONG_8BIT, &val) != 1) {

Does DEF_FMT_LONG_8BIT make its destination long, meaning 64 bits in a 64 bit build?

val is only 32 bits.

If you don't understand, post the definition of DEF_FMT_LONG_8BIT and I can explain more specifically.

In architectures where uint32 is the same size as long, this code works. In architectures where long is bigger, this code clobbers the low half of a register saved by the entry into this function.

It then returns to the caller with the wrong value in a register. Whether/how that matters depends on details of the optimization in that calling function. As I started to explain in post #24, I had deduced that the optimizer placed an extra copy of the address of num in a callee saved register that happens to be the register clobbered by the bug in get.

amir1981 · 07-26-2010, 03:48 PM

// constants: 8-bit version of the default format strings (for efficiency)
//
const char SysString:

EF_FMT_VOIDP_8BIT[] = "%p";
const char SysString:

EF_FMT_ULONG_8BIT[] = "%lu";

could it be the problem? Actually your comment make sense to me but I wonder if this could make segmentation fault?

amir1981 · 07-26-2010, 03:51 PM

I think I can correct this part and then I will report the result here, thanks your comment is really make sense now after some thinking

johnsfine · 07-26-2010, 03:51 PM

Quote:

Originally Posted by amir1981

// constants: 8-bit version of the default format strings (for efficiency)
//
const char SysString:

EF_FMT_VOIDP_8BIT[] = "%p";
const char SysString:

EF_FMT_ULONG_8BIT[] = "%lu";

could it be the problem? Actually your comment make sense to me but I wonder if this could make segmentation fault?

In the case in question it seems to be using
DEF_FMT_LONG_8BIT, but you posted two other format strings.

Anyway, this is the problem. There is a similar problem in more than one of your overloads of get. So fix it in each place, not just in the one that is causing this specific seg fault.

johnsfine · 07-26-2010, 04:03 PM

Quote:

Originally Posted by amir1981

I think I can correct this part

If you just need portability across modern 32 bit and 64 bit architectures, you can just drop the l from your 32 bit format strings: Use %u instead of %lu and use %d for signed 32 bit.