Introduce debugging symbols #915

2022-11-02T00:12:13+09:00

ghallak commented

2022-11-02 00:12:13 +09:00

(Migrated from gitlab.com)

No description provided.

zxq9 commented

2023-02-05 21:40:09 +09:00

(Migrated from gitlab.com)

Created by: radrow

Why lookup all?

*Created by: radrow* Why lookup all?

zxq9 commented

2023-02-05 21:51:26 +09:00

(Migrated from gitlab.com)

Created by: radrow

There should be annotation to the occurence of state variable. This applies to all builtins, so maybe resolution of builtin variables should be adjusted.

*Created by: radrow* There should be annotation to the occurence of `state` variable. This applies to all builtins, so maybe resolution of builtin variables should be adjusted.

zxq9 commented

2023-02-05 21:51:39 +09:00

(Migrated from gitlab.com)

Created by: radrow

Ann should point to put, not to the argument

*Created by: radrow* Ann should point to `put`, not to the argument

zxq9 commented

2023-02-05 21:52:39 +09:00

(Migrated from gitlab.com)

Created by: radrow

Should take annotations

*Created by: radrow* Should take annotations

zxq9 commented

2023-02-05 21:54:53 +09:00

(Migrated from gitlab.com)

Created by: radrow

Why not list only the needed ones?

*Created by: radrow* Why not list only the needed ones?

zxq9 commented

2023-02-05 21:59:33 +09:00

(Migrated from gitlab.com)

Created by: radrow

I would split it two lines, as you are doing two things in one line: saving argument locations and marking function entry

*Created by: radrow* I would split it two lines, as you are doing two things in one line: saving argument locations and marking function entry

zxq9 commented

2023-02-05 22:02:36 +09:00

(Migrated from gitlab.com)

Created by: radrow

To consider: maybe push could take Ann and take care of dbg_loc itself?

*Created by: radrow* To consider: maybe `push` could take `Ann` and take care of `dbg_loc` itself?

zxq9 commented

2023-02-05 22:14:32 +09:00

(Migrated from gitlab.com)

Created by: radrow

Also applies to similar instructions

*Created by: radrow* Also applies to similar instructions

zxq9 commented

2023-02-05 22:22:06 +09:00

(Migrated from gitlab.com)

Created by: radrow

Can you remind me why we don't keep column number?

*Created by: radrow* Can you remind me why we don't keep column number?

zxq9 commented

2023-02-05 22:24:43 +09:00

(Migrated from gitlab.com)

Created by: radrow

I don't think you need a dbg_loc here, as it will be added by tuple

*Created by: radrow* I don't think you need a `dbg_loc` here, as it will be added by `tuple`

zxq9 commented

2023-02-05 22:28:22 +09:00

(Migrated from gitlab.com)

Created by: radrow

Would it work if I put a breakpoint on a case arrow?

switch(x)
  [] =>  // I want a breakpoint here 
    2137
``

*Created by: radrow* Would it work if I put a breakpoint on a case arrow? ``` switch(x) [] => // I want a breakpoint here 2137 ``

zxq9 commented

2023-02-05 22:34:00 +09:00

(Migrated from gitlab.com)

Created by: radrow

Use get_value/3 to handle default

*Created by: radrow* Use `get_value/3` to handle default

zxq9 commented

2023-02-05 22:35:14 +09:00

(Migrated from gitlab.com)

Created by: radrow

what is %? Even if it's not your invention, I think it's frail to depend on how variables are named. I would use some {atom, _} instead?

*Created by: radrow* what is `%`? Even if it's not your invention, I think it's frail to depend on how variables are named. I would use some `{atom, _}` instead?

zxq9 commented

2023-02-05 22:45:36 +09:00

(Migrated from gitlab.com)

Created by: radrow

How does it handle variables defined in cases or in alts? I mean,

let x = 0
let y = 1
switch(999)
  x =>
    let y = x + 1

x == 0
y == 0

*Created by: radrow* How does it handle variables defined in cases or in alts? I mean, ``` let x = 0 let y = 1 switch(999) x => let y = x + 1 x == 0 y == 0 ```

zxq9 commented

2023-02-05 22:47:58 +09:00

(Migrated from gitlab.com)

Created by: radrow

Are subsequent let overwrites considered?

let x = 21
let x = 37
x == 37

*Created by: radrow* Are subsequent `let` overwrites considered? ``` let x = 21 let x = 37 x == 37 ```

zxq9 commented

2023-02-05 22:53:35 +09:00

(Migrated from gitlab.com)

Created by: radrow

There is a little issue with instructions that end basic blocks. As DBG_UNDEF is not an end_bb instruction (and shouldn't be), it has to be called before the actual ending instruction. This may create a case in which we may prematurely loose information about variable locations:

DBG_DEF "x" 1
...
DBG_UNDEF "x"
RETURNR {var, 1}  // We forgot that 1 is "x"!

As I think it should not be an issue if DBG_LOC is always put before DBG_UNDEF and always after DBG_DEF. I would double check if that's the case.

We should look at it closer.

*Created by: radrow* There is a little issue with instructions that end basic blocks. As `DBG_UNDEF` is not an `end_bb` instruction (and shouldn't be), it has to be called before the actual ending instruction. This may create a case in which we may prematurely loose information about variable locations: ``` DBG_DEF "x" 1 ... DBG_UNDEF "x" RETURNR {var, 1} // We forgot that 1 is "x"! ``` As I think it should not be an issue if `DBG_LOC` is always put **before** `DBG_UNDEF` and always **after** `DBG_DEF`. I would double check if that's the case. We should look at it closer.

zxq9 commented

2023-02-05 23:01:49 +09:00

(Migrated from gitlab.com)

Created by: radrow

Pure is used to mark instructions that don't alter the chain, so they can be moved around more freely. I think debug instructions should be considered Impure, so they are protected from compiler mangling.

none is fine, it means that the instruction does not write to anywhere nor alter our position

*Created by: radrow* `Pure` is used to mark instructions that don't alter the chain, so they can be moved around more freely. I think debug instructions should be considered `Impure`, so they are protected from compiler mangling. `none` is fine, it means that the instruction does not write to anywhere nor alter our position

zxq9 commented

2023-02-05 23:06:21 +09:00

(Migrated from gitlab.com)

Created by: radrow

Why not just check if only the last instruction is of the same loc?

*Created by: radrow* Why not just check if only the last instruction is of the same loc?

zxq9 commented

2023-02-05 23:07:31 +09:00

(Migrated from gitlab.com)

Created by: radrow

At this point I would assume DBG_LOCs are added correctly, and we only want to get rid of redundancies.

*Created by: radrow* At this point I would assume DBG_LOCs are added correctly, and we only want to get rid of redundancies.

zxq9 commented

2023-02-05 23:16:39 +09:00

(Migrated from gitlab.com)

Created by: radrow

Actually, do we have to hash anything when compiling with debug symbols?

*Created by: radrow* Actually, do we have to hash anything when compiling with debug symbols?

ghallak commented

2023-03-08 18:04:51 +09:00

(Migrated from gitlab.com)

Fixed here dca4466a9b9dd9830a1f941178068535ddfe16d3

ghallak commented

2023-03-08 18:46:39 +09:00

(Migrated from gitlab.com)

Done here 59aecac0a1a8cefca29cfe96ab2086396e21ac4f

ghallak commented

2023-03-08 19:23:40 +09:00

(Migrated from gitlab.com)

Fixed here 092eecc2ace01476ef47ae709e35853088ce437d

ghallak commented

2023-03-09 17:59:38 +09:00

(Migrated from gitlab.com)

Changed to Impure here c721eccf11fed7bca418f411a249e849841db7be

Changed to `Impure` here c721eccf11fed7bca418f411a249e849841db7be

ghallak commented

2023-03-14 14:07:08 +09:00

(Migrated from gitlab.com)

Done here e8975c8fd662a5e1d8ea7836e5bedb4803d71f20

ghallak commented

2023-03-14 15:11:07 +09:00

(Migrated from gitlab.com)

Fixed here 85cc37e29463d2277b8193725b817882079e156c

ghallak commented

2023-03-14 15:11:19 +09:00

(Migrated from gitlab.com)

Fixed here 85cc37e294

Fixed here https://github.com/aeternity/aesophia/commit/85cc37e29463d2277b8193725b817882079e156c

ghallak commented

2023-03-14 15:11:37 +09:00

(Migrated from gitlab.com)

Fixed here 85cc37e294

Fixed here https://github.com/aeternity/aesophia/commit/85cc37e29463d2277b8193725b817882079e156c

ghallak commented

2023-03-14 15:16:05 +09:00

(Migrated from gitlab.com)

It is not needed for setting breakpoints. Why should we have it?

ghallak commented

2023-03-14 15:20:56 +09:00

(Migrated from gitlab.com)

Fixed here ee717c621c3b8846cda6b35aab51f2c18c207ea0

ghallak commented

2023-03-14 15:35:07 +09:00

(Migrated from gitlab.com)

Why keep passing Ann down to functions that don't need it while we can end things in to_scode1?

Why keep passing `Ann` down to functions that don't need it while we can end things in `to_scode1`?

ghallak commented

2023-03-14 16:09:29 +09:00

(Migrated from gitlab.com)

what is %?

This is for fresh names (they have the form %n like %0 or %1). We don't want to do have DBG_DEF and DBG_UNDEF for this fresh names.

I would use some {atom, _} instead?

I don't get your suggestion. Can you explain this more?

>what is %? This is for fresh names (they have the form `%n` like `%0` or `%1`). We don't want to do have `DBG_DEF` and `DBG_UNDEF` for this fresh names. >I would use some {atom, _} instead? I don't get your suggestion. Can you explain this more?

ghallak commented

2023-03-14 16:55:11 +09:00

(Migrated from gitlab.com)

It will not work, but do we really need that?

ghallak commented

2023-03-14 17:14:16 +09:00

(Migrated from gitlab.com)

Setting a breakpoint on the pattern (before the arrow) will cause a break before the first instruction after the arrow is executed.

In which case would you need to break before the instructions before the arrow are executed?

Setting a breakpoint on the pattern (before the arrow) will cause a break before the first instruction after the arrow is executed. In which case would you need to break before the instructions before the arrow are executed?

ghallak commented

2023-03-14 17:53:28 +09:00

(Migrated from gitlab.com)

Why not just check if only the last instruction is of the same loc?

Do you mean checking for each DBG_LOC instruction at position I for the instruction at position I - 1?

If that's what mean, then it would only work for the following case:

DBG_LOC N
DBG_LOC N
instrucions

When multiple expressions are on the same line, the generated instructions would look like this:

DBG_LOC N
instructions
DBG_LOC N
instructions

This is why I had to use a set.

>Why not just check if only the last instruction is of the same loc? Do you mean checking for each `DBG_LOC` instruction at position `I` for the instruction at position `I - 1`? If that's what mean, then it would only work for the following case: ``` DBG_LOC N DBG_LOC N instrucions ``` When multiple expressions are on the same line, the generated instructions would look like this: ``` DBG_LOC N instructions DBG_LOC N instructions ``` This is why I had to use a `set`.

ghallak commented

2023-03-14 18:41:29 +09:00

(Migrated from gitlab.com)

Yes, there will be a DBG_DEF and DBG_UNDEF for each of them:

{'DBG_DEF',{immediate,"x"},{var,0}},
{'DBG_DEF',{immediate,"x"},{var,1}},
.
.
{'DBG_UNDEF',{immediate,"x"},{var,1}},
{'DBG_UNDEF',{immediate,"x"},{var,0}},

Yes, there will be a `DBG_DEF` and `DBG_UNDEF` for each of them: ``` {'DBG_DEF',{immediate,"x"},{var,0}}, {'DBG_DEF',{immediate,"x"},{var,1}}, . . {'DBG_UNDEF',{immediate,"x"},{var,1}}, {'DBG_UNDEF',{immediate,"x"},{var,0}}, ```

ghallak commented

2023-03-14 19:08:52 +09:00

(Migrated from gitlab.com)

In which case would you see something like RETURNR {var, 1}? The return result is pushed to the stack and RETURN is used instead of RETURNR most of the times.

Can you point to an example that would produce the case that you're talking about?

In which case would you see something like `RETURNR {var, 1}`? The return result is pushed to the stack and `RETURN` is used instead of `RETURNR` most of the times. Can you point to an example that would produce the case that you're talking about?

ghallak commented

2023-03-14 19:46:18 +09:00

(Migrated from gitlab.com)

I have just realized that there are no DBG_DEF and DBG_UNDEF instructions for the x that is to the left of the arrow. I will see if I can fix this.

I have just realized that there are no `DBG_DEF` and `DBG_UNDEF` instructions for the `x` that is to the left of the arrow. I will see if I can fix this.

zxq9 commented

2023-03-19 04:32:06 +09:00

(Migrated from gitlab.com)

Created by: radrow

When you want to see what the pattern variables were bound to

*Created by: radrow* When you want to see what the pattern variables were bound to

zxq9 commented

2023-03-19 04:35:00 +09:00

(Migrated from gitlab.com)

Created by: radrow

If you have a complex expression such as zeroth(first(), second(x), third(y) + 3215) you may be able to put breakpoints more selectively. Here for example just before second is executed, when other functions can modify the state.

But maybe let's leave it for later

*Created by: radrow* If you have a complex expression such as `zeroth(first(), second(x), third(y) + 3215)` you may be able to put breakpoints more selectively. Here for example just before `second` is executed, when other functions can modify the state. But maybe let's leave it for later

zxq9 commented

2023-03-19 04:41:13 +09:00

(Migrated from gitlab.com)

Created by: radrow

Ah I see. Maybe then have a function to tell if a variable is fresh? This piece of code looks super random if you don't know it.

What I meant is to distinguish things like {user_var, _} and {system_var, _} (naming whatever), so it would be clear that we are handling a variable added by the system. But I think having a function to check it based on name would be simpler for now.

*Created by: radrow* Ah I see. Maybe then have a function to tell if a variable is fresh? This piece of code looks super random if you don't know it. What I meant is to distinguish things like `{user_var, _}` and `{system_var, _}` (naming whatever), so it would be clear that we are handling a variable added by the system. But I think having a function to check it based on name would be simpler for now.

zxq9 commented

2023-03-19 04:50:45 +09:00

(Migrated from gitlab.com)

Created by: radrow

  entrypoint f(x)  : int =
    x

compiles to

FUNCTION f( integer) : integer
  ;; BB : 0
          RETURNR arg0

I just want to be sure if we cover such cases properly. Even though we may not use RETURNR extensively now, we might use it later for whatever reason --- let's be prepared for such possibility.

*Created by: radrow* ``` entrypoint f(x) : int = x ``` compiles to ``` FUNCTION f( integer) : integer ;; BB : 0 RETURNR arg0 ``` ----------- I just want to be sure if we cover such cases properly. Even though we may not use `RETURNR` extensively now, we might use it later for whatever reason --- let's be prepared for such possibility.

zxq9 commented

2023-03-19 04:55:06 +09:00

(Migrated from gitlab.com)

Created by: radrow

That's why I wanted columns, but fair for now

*Created by: radrow* That's why I wanted columns, but fair for now

ghallak commented

2023-03-22 20:23:06 +09:00

(Migrated from gitlab.com)

Done here ad382302f84c895f71d2ee9ad3e4d2e59b5b4f0d.

ghallak commented

2023-03-22 22:37:47 +09:00

(Migrated from gitlab.com)

I'm turning off all optimizations including swap_push optimization for debugging compilation, so they won't happen, and the generated fate code won't have a RETURNR arg0. But it's something to consider in the future.

I'm turning off all optimizations including swap_push optimization for debugging compilation, so they won't happen, and the generated fate code won't have a `RETURNR arg0`. But it's something to consider in the future.

zxq9 commented

2023-04-07 16:34:12 +09:00

(Migrated from gitlab.com)

Created by: radrow

If you insist on not covering it now, make at least an issue for that

*Created by: radrow* If you insist on not covering it now, make at least an issue for that

ghallak commented

2023-04-08 18:02:53 +09:00

(Migrated from gitlab.com)

This was solved here 959371e82467bc26a4f0af535c4dfd3a84a89bb4.

zxq9 commented

2023-04-26 07:12:01 +09:00

(Migrated from gitlab.com)

Created by: radrow

Make a release and reference it

*Created by: radrow* Make a release and reference it

zxq9 commented

2023-04-26 07:37:37 +09:00

(Migrated from gitlab.com)

Created by: radrow

Review: Commented

I would like to play with it in REPL before I approve. There are some issues there though, I can spend some time after it works. But otherwise it looks fine.

*Created by: radrow* **Review:** Commented I would like to play with it in REPL before I approve. There are some issues there though, I can spend some time after it works. But otherwise it looks fine.

zxq9 commented

2023-05-25 19:27:28 +09:00

(Migrated from gitlab.com)

Created by: hanssv

FAnn or get_fann(Body)? This looks weird

*Created by: hanssv* FAnn or get_fann(Body)? This looks weird

zxq9 commented

2023-05-25 19:29:59 +09:00

(Migrated from gitlab.com)

Created by: hanssv

Review: Dismissed

Looks good 👍

*Created by: hanssv* **Review:** Dismissed Looks good 👍

ghallak commented

2023-05-25 21:23:05 +09:00

(Migrated from gitlab.com)

Fixed this here 30def26aaeee27a6b3c7dd457c7d452fc9493b01.

zxq9 commented

2023-06-12 22:17:06 +09:00

(Migrated from gitlab.com)

Created by: radrow

Review: Approved

*Created by: radrow* **Review:** Approved

zxq9 commented

2023-06-12 23:10:30 +09:00

(Migrated from gitlab.com)

Created by: hanssv

Review: Approved

*Created by: hanssv* **Review:** Approved

zxq9 commented

2023-06-13 20:36:48 +09:00

(Migrated from gitlab.com)

Merged by: ghallak at 2023-06-13 11:36:48 UTC

*Merged by: ghallak at 2023-06-13 11:36:48 UTC*

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: QPQ-AG/sophia#915