0

I'm trying to do something like this - copy the top N elements of the stack to an array. I want to use it to define the invokevirtual, invokespecial, invokestatic, invokeinterface, and invokedynamic instructions for a Java Ahead-Of-Time Compiler. The stack is a linked list and __pop() unwinds and returns the top of the stack.

    public : void __sipop(){
        topframe = topframe->prev;
    }
    public : void __longpop(){
        topframe = topframe->prev->prev;
    }
    public : jvalue __pop(){
        //also shared with bytecode
        jvalue value = topframe->value;
        if(topframe->type == 'J' || topframe->type == 'D'){
            __longpop();
        } else{
            __sipop();
        }
        return value;
    }
    public : jvalue* __extract(int count){
        jvalue extracted [count];
        for(int i = 0; i < count; i++){
            extracted[count - i - 1] = __pop();
        }
        return extracted;
    }

Will my implementation crash at runtime?

Jessie Lesbian
  • 1,273
  • 10
  • 14
  • 5
    are you writing a C++ implementation? Because [names starting with double underscores (`__`) are reserved](https://stackoverflow.com/q/228783/995714) – phuclv Jun 08 '20 at 08:23
  • 1
    `jvalue extracted [count];` - looks like a VLA and is not allowed in standard C++ – Ted Lyngmo Jun 08 '20 at 08:33

2 Answers2

3

Will my implementation crash at runtime?

Maybe. You exhibit Undefined Behaviour at least in:

jvalue* __extract(int count){
    jvalue extracted [count];
    for (int i = 0; i < count; i++){
        extracted[count - i - 1] = __pop();
    }
    return extracted;
}

Your function returns a pointer to a local variable whose lifetime ends as the function returns. For additional information, you should read this excellent answer on Sotack Overflow: Can a local variable's memory be accessed outside its scope? (tl; dr: no).

The simplest solution would be to return a vector:

#include <vector> // preferably in the first lines of your header file (.hpp)

std::vector<jvalue> extract(int count)
{
    auto extracted = std::vector<jvalue>(count);
    for (int i = 0; i < count; i++) {
        extracted[count - i - 1] = __pop();
    }
    return extracted;
}

You may also be interested by std::generate.

Additionally, as mentioned in the comments, names staring by two underscores (__) are reserved to the implementation.

Unrelated note, I understand your wish to feel comfortable in C++ by mimicking aspects of the Java language, but you should write idiomatic C++ and not repeat the access modifier (public) before each member function.

YSC
  • 38,212
  • 9
  • 96
  • 149
  • I changed it to ````jvalue* extracted = new jvalue[count];```` and my GCC compiler stopped warning me about it. – Jessie Lesbian Jun 08 '20 at 08:46
  • 4
    @JessieLesbian You shouldn't unless you have a good reason to avoid vectors. You're basically opening a can of worms by recurring to dynamic memory. And if you do, you **must** learn how to handle such memory, how and when to use `delete[]`, what _memory ownership_ means, what RAII is, etc. – YSC Jun 08 '20 at 08:51
  • 1
    @JessieLesbian Don't do that - you loose the size information and need to manually `delete[]` the allocated memory. Prefer a `std::vector`. – Ted Lyngmo Jun 08 '20 at 08:51
  • First, i'm avoiding vectors since I have to pass it to JNI. Second, the calling method knows exactly how long the array is going to be. – Jessie Lesbian Jun 08 '20 at 08:55
  • 1
    @JessieLesbian You can create a Java ArrayList from a C++ vector: https://stackoverflow.com/q/7776800/5470596 – YSC Jun 08 '20 at 08:58
  • I mean, I'm dealing with this sort of signature: ````void (JNICALL *CallStaticVoidMethodA) (JNIEnv *env, jclass cls, jmethodID methodID, const jvalue * args);````, which don't like vectors. – Jessie Lesbian Jun 08 '20 at 09:00
  • 1
    @JessieLesbian Anyway, this is another question you should ask as a new question. There might be solutions combining the best of the two worlds (e.g. you can access a vector's memory with its `.data` member function). – YSC Jun 08 '20 at 09:02
2

Yes. Returning the address of a stack-local object (extracted) is undefined behavior. Return a heap-allocated array (auto extracted = new jvalue[count];) or std::vector<jvalue> instead.

Botje
  • 26,269
  • 3
  • 31
  • 41