Segmentation fault when compiling with icpc

[David_K_8]

Hello,

I ran into a strange problem with icpc when compiling the attached C++ code. When I compile with icpc -O0 icpc_segfault.cpp -o icpc_segfault the program crashes with a segmentation fault. If I compile with any other optimization flag or with clang++ or g++, the program runs fine.

I can't see any UB in the code, so my only other guess is that it is a problem with icpc. The attached code is a stripped down version of the actual program. I can "fix" the segfault in the attached code in several ways (e.g., add a const data member to the FormatList class, or declare pointers volatile), but these "fixes" don't work in the actual program. This just makes me think even more so that it's a compiler problem and not UB.

The target architecture is intel64 and I tried several versions of icpc (16.0.4, 17.0.5, 18.0.3, 19.0.5.281).

Any help on this is appreciated. It can very well be that I just don't see the UB in the code.

[jimdempseyatthecove]

Correct me if I am wrong. Your current function:

FormatList1 makeFormatList(const int& arg) {
  return FormatList1(arg);
}

is performing a ctor for FormatList1 and in the process constructing an unnamed, stack based object local to makeFormatList, and which is implicitly destroyed upon return from FormatList1.

The second "This would be fine..." does the same except the dtor won't be called until the return from main IOW remains valid until then.

Jim Dempsey

[David_K_8]

Dear Jim,

Yes, you are correct. The code I posted is a simplified version of the tinyformat library available at https://github.com/c42f/tinyformat/blob/master/tinyformat.h. The makeFormatList function in this library is meant to hide the FormatList1 class from users.

But I think I get where you are going at. The returned FormatList1 object holds a pointer to the memory allocated for m_formatterStore in the unnamed, stack based FormatList1 object. Right?

[jimdempseyatthecove]

As far as the CPU is concerned the return is the address of (pointer to) the object that was created within the stack context of the makeFormatList function. In C++ speak the return is an object... but this object is destroyed as well as no longer in scope. The following could have been used:

FormatList1* makeFormatList(const int& arg) {
  return new FormatList1(arg);
}

or

FormatList1 makeFormatList(const int& arg) {
  return *(new FormatList1(arg)); // *** bad style
}

the first method would require your calling code to use -> in lieu of . However, it should also have saved the pointer for later disposition.

The second method could work but, the caller would have to do an un-standard method to delete the object (assuming you want to avoid a memory leak). You could also have the second function returning FormatList1& (with same issue about deleting it sometime later).

Jim Dempsey

[Viet_H_Intel]

Can you compile at higher level than O0 while we look into this issue?

[David_K_8]

Thank you both for your answers and suggestions.

@Viet: Yes, I can compile at higher level than O0, but I think Jim convinced me that it's not a compiler problem but rather UB.

@Jim: I think the cleanest option is to add a copy ctor to FormatList1:

  FormatList1(const FormatList1& other)
      : FormatList(&m_formatterStore[0], 1),
        m_formatterStore { other.m_formatterStore[0] } {}

[jimdempseyatthecove]

Given how you used the makeFormatList in the dummy program, the object will be: a) constructed on stack of makeFormatList, copied to  temporary object in caller (main), immediately deleted in makeFormatList, the (temporary) copy in main will immediately be operated on by the member functions and otherwise be unreachable/unusable for the remainder of the run of the program. IMHO this is not a clean way to program.

If you intend to program this way (create and immediately use objects) consider making the member variables and functions static .OR. create a global object to perform your formatting.

Jim Dempsey

[David_K_8]

I very much agree with that assessment, Jim. In the context of the library, however, I think the maintainers made a deliberate choice to use this pattern to hide a variadic template class from users.

[jimdempseyatthecove]

RE: variadic templates and templates in general (non-static)

Back in the 1990's (Borland C++), though pre-variadic, the compiler and linker worked together such different source files using a common header and expanding a template would generate identical expanded template functions in different .obj files, AND in which the Linker would permit the linkage without complaint provided the generated code was the same. Now, in the modern/improved age, the source files including these templates must inline them or make them static to avoid duplicate symbols. IMHO this adds to code bloat. Back to the good old days.....

Jim Dempsey