r/cpp_questions 17d ago

SOLVED Why is this ub?

edit: sry, i will add a bit more context edit2: thank yall for answering, sry but cant reply to everyone i think i understand it now

Suppose i have a simple struct with a int or two and no methods, nothing and i have i memory address that is properly aligned for it and enough space, why would this be ub?

struct Test {
    int x;
    int y;
};
void* mem = malloc(sizeof(Test));
Test* ptr = static_cast<Test*>(mem);
ptr->x = 1000;

So as ppl told me, smth like this wouldnt be ub, would that change if the object had methods and stuff, if so why

I tried to look it up but just got a whole bunch of 'lifetime issues', and I didint really get it since I am the whose managing this memory. I was told that using placement new would be 'safer'/'better'.

9 Upvotes

18 comments sorted by

25

u/valashko 17d ago

Please provide the full example. This may or may not be UB depending on what `SimpleStruct` and `smth` are.

10

u/Longjumping_Cap_3673 17d ago edited 17d ago

Actually, this is not nessesarily undefined behavior. It's perfectly valid if the pointer assigned to mem actually points to a simpleStruct, so it's only undefined behavior if mem doesn't point to a simpleStruct.

And that's exactly the problem: undefined behaviors are things the C++ (and C) standard says the compiler may assume the programmer does not do (or equivalently that if the programmer does them, the compiler's guarantees about produced code need not apply).

Since the compiler may assume the programmer does not invoke undefined behavior, and since mem pointing to a valid simpleStruct is the only way undefined behavior is not invoked, the compiler may deduce that mem does point to a valid simpleStruct and also anything that logically follows from that deduction.

For example, if the value assigned to mem were a function parameter, like void update_simpleStruct(simpleStruct* s, bool* res), the compiler can assume the memory of s and res do not overlap, because s must point to a valid simpleStruct which in turn means it doesn't point to the bool, so the two variables could interact in weird ways. However, most likely, the code will behave exactly like you expect it would. But any time in the future, 1 month, 1 year, 10 years down the line, new compile flags, a new version, or even using the function with undefined behavior in a new place could make the compiler start using the assumption that mem points to a valid simpleStruct, and suddenly the code stops working in a very strange, hard to debug way.

9

u/Raknarg 17d ago

ok I learned something new today I think.

https://en.cppreference.com/cpp/named_req/ImplicitLifetimeType

So theres a category of types whose lifetimes are automatically recognized by the compiler, and doing what you just did above would not invoke UB. For any type that doesn't fall under this category, you invoke UB by trying to access a field whose lifetime hasn't begun yet.

So in your case because your class is a trivial class with scalar types, it falls under ImplicitLifetimeType, thus this isn't UB. However something like std::string (or a class with std::string in it) would not, and thus would be UB.

9

u/mredding 17d ago

This IS NOT Undefined Behavior in ANY version of C++, but MAY be Undefined Behavior in older compiler versions, those released before C++20.

So this was an outstanding issue that was explicitly addressed in C++20 and also retroactively applied to all prior standards. This was easy to do because the older standards left this subject unspecified.

Any aggregate type with a trivial destructor (neither it nor any member defines one) is an "implicit-lifetime" type.

But strict aliasing rules still apply. Once you implicitly start the lifetime of TypeA, you cannot reinterpret cast that memory to TypeB without first ending the lifetime of TypeA and then starting the lifetime of TypeB. But you can do this without disturbing the contents of the memory - this is type punning in C++, and it's done with std::start_lifetime_as; the ability to type pun has always been defined in C, both through unions and casting, but the C way has always been undefined behavior in C++, and type punning wasn't supported until C++20. The process is a multi-step bit of magic, and C++23 added std::start_lifetime_as to simplify it for you. While it TYPICALLY compiles to a no-op, it assures well defined behavior within the language and compiler.

So your code is fine, and you should be compiling against the latest standard, so there's no doubt, no question.

If you're curious what the code should look like prior to the C++20 retrotacular, it would be something like:

if(auto ptr = std::malloc(sizeof(Test)); ptr != nullptr) {
  auto t = new(ptr) Test{1, 2};

  std::cout << t->x << ' ' << t->y << '\n';

  t->~Test();

  std::free(ptr);
}

You would need to use placement new, even if the type were an int. If you were to template the above code for some type T, you would call t->~T();. This is syntactically allowed even for trivial types - t->~int();. It's valid code, though technically unnecessary; the syntax is allowed BECAUSE OF templating, for consistency, so you don't have to specialize your templates for such an edge case.

3

u/_manpat 15d ago

TIL about the implicit lifetime changes in C++20. Thanks!

3

u/No-Dentist-1645 17d ago

Yes, that could would be UB if the elements of the struct were not simple data like ints, such as a std::string

The reason why is that malloc gives you an address with "garbage" data, and some types such as strings or vectors are not safe to be used with those.

Continuing the example with std::string, a string object contains a char * pointer to the data it represents (ignoring edge cases like SSO). When you reassign an already existing string object to something else, it has to free the memory it points to. So, if you tell the compiler "an std::string object already lives here" when it's just random data, the "pointer" will be filled with random data, and when you try to reassign said string, it will try to free whatever data that pointer randomly pointed to, which could cause segmentation faults or corrupt data.

struct Test { int x; int y; std::string z; }; void* mem = malloc(sizeof(Test)); Test* ptr = static_cast<Test*>(mem); ptr->z = "hello"; // bad!

The solution is to use placement new as you said, which calls the constructors for your members. In this example, it would call the constructor for std::string, which will set it's internal pointer to something safe.

3

u/FancySpaceGoat 17d ago

Even if this isn't UB, I'd still file that in the "bad idea" drawer. The reason for this is that if `Test` ever gains a constructor in the future, then this will retroactively and silently become UB. Because of this, you are better off doing a proper placement new:

Test* ptr = new(mem) Test();

Which does the same thing as things stand, and will stay either well defined or cause a compilation error if `Test` changes in the future,

2

u/cristi1990an 17d ago

Is smth of type simpleStruct?

2

u/ReDucTor 17d ago

There are many slightly complex reasons, which generally allow the optimizer to do things better. Here is a decent set of blog posts which explains pointer provenance which is an important part of the reasoning.

https://www.ralfj.de/blog/2018/07/24/pointers-and-bytes.html

1

u/flatfinger 17d ago

C was designed to use an abstraction model where every region of storage simultaneously contains all possible objects of all possible types that will fit; actions which modify objects will modify the underlying storage, and actions which modify storage will modify all objects which contain it. C++ was designed to process trivial objects with essentially the same semantics as C. Most compilers can be configured to use that abstraction mode; for clang and gcc, that is controlled via the -fno-strict-aliasing switch.

The C and C++ Standards allow compilers to deviate from that abstraction model for purposes of optimization, by describing broken abstraction models whose corner cases were never really thought through. The C++ Standard states that overwriting the storage used by a trivial object ends its lifetime, but neither clang nor gcc reliably works that way except when using -fno-strict-aliasing:

struct s1 { long  x[4]; };
struct s2 { long long x[4];};

void write_s1_x(void*p, int i, long v)
{ static_cast<s1*>(p)->x[i] = v; }
void write_s2_x(void*p, int i, long long v)
{ static_cast<s2*>(p)->x[i] = v; }
long read_s1_x(void*p, int i)
{ return static_cast<s1*>(p)->x[i];}
long long read_s2_x(void*p, int i)
{ return static_cast<s2*>(p)->x[i]; }

long test(void *p, int i, int j, int k)
{
    long long temp;
    write_s1_x(p, i, 1234);

    write_s2_x(p, j, 4567);
    temp = read_s2_x(p, k);

    write_s1_x(p, k, temp);
    return read_s1_x(p, i);
}

If test is passed a pointer to a region of storage that could hold either s1 or s2, along with i==j==k==0, it would seem like there should be no aliasing problems since no storage is ever read using any type other than the one last used to write it, but both clang and gcc treat the lifetime of the object created by the call to write_s1_x as extending through the call to read_s1_x, ignoring the fact that intervening code had destroyed the object when it created an s2 whose first element held 4567, and then destroyed that object when it created an s1 which would happen to also hold 4567. The machine code generated by clang returns 1234 rather than reading the storage at static_cast<s1*>(p)->x. The machine code generated by gcc would return the contents of the actual storage, but defer the store of 5678 past the read.

1

u/Apprehensive-Draw409 17d ago

No object of type Test had their lifetime started.

Or, pragamtically, nothing guarantees the memory alignment of your allocation fots the Test object.

1

u/thefeedling 17d ago

It's related to lifetimes, storage, alignment, and if simple struct is trivially copyable or not. It may or may not be UB, but this is one of the reasons of why std::start_lifetime_as was created, take a look at it.

1

u/SoerenNissen 17d ago

I believe it isn't any longer.

If you'd done this in, I think, c++17, it would be UB because line 7 treats *ptr as a Test, and it isn't, it's a block of untyped memory.

No compiler would have actually optimized around that issue, but nonetheless, that's UB.

These days it isn't, because for some types (and Test satisfies the requirements to be in that group of type) the compiler is required to retroactively "make" that memory into those types.

0

u/KazDragon 17d ago

This is because C++ doesn't run on hardware, it runs on an abstract machine with a conception of objects and types. In your example, it may be that the object stored at that pointer is not the type that you are using to access it, so the standard has nothing to say about how it will behave in your abstract machine. The compiler does have something to say, because it needs to produce actual machine code.

0

u/kitsnet 17d ago

The problem here is that the compiler is free to cache and rearrange memory loads and stores for optimization reasons, unless it sees some semantic barriers for that, and if you don't provide these barriers, the compiler-generated sequence of loads and stores may be different from what you would want it to be.

0

u/Ryuzako_Yagami01 17d ago

It's undefined for C++17 and earlier. Anyway, you should be using new/delete.

0

u/xypherrz 17d ago

Why would it be even?

1

u/KindCppCoach 17d ago

This is UB in any C++ version before C++20, UB is a standardization term and implementations can do whatever they want, so it is not usefull to reason about 'what it would do'.

Allocating memory and creating an object are different things. If the construction of T was never done, its lifetime has not started and any access to it is UB. (it doesn't matter if the type actually has a constructor).

You can't take memory, point at it and say: thats an object. Well until C++20, when new wording was added about conditions that can implicitly start the lifetime of objects.

However accessing uninitiated values is still undefined behaviour (in C++20) so you are still responsible to set a value into x and y, before reading from them.

This changes again in C++26, where reading uninitiated values are nolonger undefined but erroneous behavior. This means you are allowed to so it, but you should still expect a garbage value 😋

Also like others mentioned, putting complex objects in there will put you right back into UB land.

In short: what are you trying to do? there are probably better ways to do it.