Add debug configuration to build system

[mcevoypeter]

Description

Resolves #83.

Testing

$ bazel build :urbitdbg

When running the resulting binary under gdb, no arguments are optimized out, as expected with an optimization level of -O0 (set by the debug configuration).

$ gdb ./urbit
GNU gdb (GDB) 12.1
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./urbit...
(gdb) list pkg/noun/allocate.h:310
305                                  ? (c3_w)(r->mat_p - r->cap_p) \
306                                  : (c3_w)(r->cap_p - r->mat_p) )
307
308     #     define  u3a_north_is_senior(r, dog) \
309                     __((u3a_to_off(dog) < r->rut_p) ||  \
310                            (u3a_to_off(dog) >= r->mat_p))
311
312     #     define  u3a_north_is_junior(r, dog) \
313                     __((u3a_to_off(dog) >= r->cap_p) && \
314                            (u3a_to_off(dog) < r->mat_p))
(gdb) list pkg/noun/urth.c:371
366     /* _cu_realloc(): hash-cons roots off-loom, reallocate on loom.
367     */
368     static ur_nref
369     _cu_realloc(FILE* fil_u, ur_root_t** tor_u, ur_nvec_t* doc_u)
370     {
371     #ifdef U3_MEMORY_DEBUG
372       c3_assert(0);
373     #endif
374
375       //  bypassing page tracking as an optimization
(gdb) set args zod
(gdb) r
Starting program: /home/tlon/code/gh/urbit/vere/urbit zod

This GDB supports auto-downloading debuginfo from the following URLs:
https://debuginfod.archlinux.org
Enable debuginfod for this session? (y or [n]) n
Debuginfod has been disabled.
To make this setting permanent, add 'set debuginfod enabled off' to .gdbinit.
~
urbit 1.18-4e89526eb1
boot: home is /home/tlon/code/gh/urbit/vere/zod
loom: mapped 2048MB
lite: arvo formula 2a2274c9
lite: core 4bb376f0
lite: final state 4bb376f0
[Detaching after fork from child process 1438063]
loom: mapped 2048MB
boot: protected loom
live: logical boot
boot: installed 351 jets
---------------- playback starting ----------------
pier: replaying events 1-16

Program received signal SIGINT, Interrupt.
                                          _me_lose_north (dog=3247830431) at pkg/noun/allocate.c:1374
1374        if ( box_u->use_w > 1 ) {
(gdb) bt
#0  _me_lose_north (dog=3247830431) at pkg/noun/allocate.c:1374
#1  0x0000000000436277 in u3a_lose (som=3247830431) at pkg/noun/allocate.c:1470
#2  0x000000000043ca1e in _ch_free_node (han_u=0x20439ad2c, lef_w=10) at pkg/noun/hashtable.c:692
#3  0x000000000043ca66 in _ch_free_node (han_u=0x207c09eac, lef_w=15) at pkg/noun/hashtable.c:700
#4  0x000000000043ca66 in _ch_free_node (han_u=0x206318b04, lef_w=20) at pkg/noun/hashtable.c:700
#5  0x000000000043cb1c in u3h_free (har_p=15708796) at pkg/noun/hashtable.c:724
#6  0x000000000044d2b1 in _cr_sing (a=3230144997, b=3223017927) at pkg/noun/retrieve.c:562
#7  0x000000000044d33d in u3r_sing (a=3227066632, b=3227884839) at pkg/noun/retrieve.c:580
#8  0x000000000043c749 in _ch_node_git (han_u=0x208225bd4, lef_w=10, rem_w=916, key=3227066632) at pkg/noun/hashtable.c:598
#9  0x000000000043c7e6 in _ch_node_git (han_u=0x2081a08e0, lef_w=15, rem_w=16276, key=3227066632) at pkg/noun/hashtable.c:611
#10 0x000000000043c7e6 in _ch_node_git (han_u=0x20840a524, lef_w=20, rem_w=737172, key=3227066632) at pkg/noun/hashtable.c:611
#11 0x000000000043c949 in u3h_git (har_p=15708722, key=3227066632) at pkg/noun/hashtable.c:646
#12 0x00000000004509cb in _cs_jam_xeno_cell (a=3227066632, ptr_v=0x7fffffffe0c0) at pkg/noun/serial.c:254
#13 0x000000000043862c in u3a_walk_fore (a=3227066632, ptr_v=0x7fffffffe0c0, pat_f=0x4508c5 <_cs_jam_xeno_atom>, cel_f=0x450996 <_cs_jam_xeno_cell>) at pkg/noun/allocate.c:2607
#14 0x0000000000450ab9 in u3s_jam_xeno (a=3241480374, len_d=0x7fffffffe130, byt_y=0x7fffffffe128) at pkg/noun/serial.c:276
#15 0x000000000043212a in _lord_writ_send (god_u=0x7ffff7cd10e0, wit_u=0x7ffff7ff70d0) at pkg/vere/lord.c:841
#16 0x00000000004322d7 in _lord_writ_plan (god_u=0x7ffff7cd10e0, wit_u=0x7ffff7ff70d0) at pkg/vere/lord.c:869
#17 0x0000000000432531 in u3_lord_play (god_u=0x7ffff7cd10e0, fon_u=...) at pkg/vere/lord.c:924
#18 0x0000000000417ded in _pier_play_send (pay_u=0x7ffff7ff7e10) at pkg/vere/pier.c:968
#19 0x000000000041802c in _pier_play (pay_u=0x7ffff7ff7e10) at pkg/vere/pier.c:1055
#20 0x0000000000418694 in _pier_on_disk_read_done (ptr_v=0x7ffff7ff7380, fon_u=...) at pkg/vere/pier.c:1218
#21 0x00000000004074d2 in _disk_read_done_cb (tim_u=0x7ffff7cd1840) at pkg/vere/disk.c:357
#22 0x000000000058e377 in uv__run_timers (loop=0xdc0a60 <default_loop_struct>) at src/timer.c:178
#23 0x000000000059297d in uv_run (loop=0xdc0a60 <default_loop_struct>, mode=UV_RUN_DEFAULT) at src/unix/core.c:393
#24 0x0000000000413d57 in u3_king_commence () at pkg/vere/king.c:925
#25 0x00000000004057cf in main (argc=2, argv=0x7fffffffe4c8) at pkg/vere/main.c:2249

[barter-simsum]

After realizing that specifying copts via the build command line switch like so: bazel build -copt="-O0" applies not just to urbit, but to all dependencies, we've opted to specify -O3 optimization for all third-party dependencies in their respective .BUILD files and to define a new build target urbitdbg to build a debug variant of urbit that will leave third party dependencies at their defined optimization level. + @mcevoypeter

[mcevoypeter]

Thanks for adding changes. Just a few small comments from me.

[mcevoypeter]

Looks good to me. As the PR author, I can't approve this. @barter-simsum, you may be able to approve it and merge it. If not, we'll need to wait on approval for @philipcmonk or someone else in @urbit/runtime.

[barter-simsum]

One last change. As @hosted-fornet suggested in #174, removing the $ in markdown instructions.

[barter-simsum]

Looks good to me. As the PR author, I can't approve this. @barter-simsum, you may be able to approve it and merge it. If not, we'll need to wait on approval for @philipcmonk or someone else in @urbit/runtime.

Will have to wait on @philipcmonk to approve. Last pusher to the branch can't approve.

[philipcmonk]

What is the flow for (1) CPU_DEBUG+MEMORY_DEBUG but also -O3 (for CI because it's fast) and (2) -O0 -g but no CPU_DEBUG+MEMORY_DEBUG (for debugging on live ships because you can't use those macros on ships created by binaries without those macros)?

[barter-simsum]

What is the flow for (1) CPU_DEBUG+MEMORY_DEBUG but also -O3 (for CI because it's fast) and (2) -O0 -g but no CPU_DEBUG+MEMORY_DEBUG (for debugging on live ships because you can't use those macros on ships created by binaries without those macros)?
@philipcmonk

(1) is easy. bazel build --copt="-DCPU_DEBUG" --copt="-DMEMORY_DEBUG" :urbit

(2) That's a good point about the loom/binary incompatibility. We should probably avoid defining debug macros with this behavior in the bazel rule and require that they be provided explicitly. Though for something like the C3DBG macro introduced in #164, we can leave this defined as it just guards additional asserts that are useful inter development.

With those changes, the command to build (2) would be

bazel build :urbitdbg

[barter-simsum]

@philipcmonk with the changes introduced in 2763de2, binary/loom compatibility breaking macros will have to be explicitly included for all build targets if they're desired

bazel build --copt="-DCPU_DEBUG" --copt="-DMEMORY_DEBUG" :urbit
Builds with -O3 -g and includes cpu/mem debug macros.

bazel build :urbitdbg
Builds with -O0 -g3, but doesn't define the CPU and MEM debug macros. May define other macros in the future but they won't be any that break binary/loom compatibility.

[philipcmonk]

Looks good!

[barter-simsum]

Not ready to merge yet. Discovered somewhat strange behavior with bazel when a copt is specified via the bazel target and also provided via the commandline switch. For instance given:

bazel build --copt="-O3" :urbitdbg && readelf --debug-dump=info bazel-bin/pkg/vere/urbitdbg | head

Observe in the output below that urbitdbg was built with both optimizations provided -O3 -O0. The last one takes precedence, which means that afaict you can't override a target's copts from the command line.

Loading: 
Loading: 0 packages loaded
Analyzing: target //:urbitdbg (0 packages loaded, 0 targets configured)
INFO: Analyzed target //:urbitdbg (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
bazel: Entering directory `/home/d/.cache/bazel/_bazel_d/223b03f966d86484fac14b7df9c36246/execroot/__main__/'
[0 / 1] [Prepa] BazelWorkspaceStatusAction stable-status.txt
bazel: Leaving directory `/home/d/.cache/bazel/_bazel_d/223b03f966d86484fac14b7df9c36246/execroot/__main__/'
Target //pkg/vere:urbitdbg up-to-date:
  bazel-bin/pkg/vere/urbitdbg
INFO: Elapsed time: 0.188s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Build completed successfully, 1 total action
Contents of the .debug_info section:

  Compilation Unit @ offset 0x0:
   Length:        0x7905 (32-bit)
   Version:       4
   Abbrev Offset: 0x0
   Pointer Size:  8
 <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
    <c>   DW_AT_producer    : (indirect string, offset: 0x7a53c): GNU C17 9.4.0 -mtune=generic -march=x86-64 -g3 -O3 -O0 -frandom-seed=bazel-out/k8-fastbuild/bin/pkg/vere/_objs/urbitdbg/main.o
    <10>   DW_AT_language    : 12	(ANSI C99)

If all we want to do is ensure third party dependencies are built with -O3 (or -O2 in some cases), then we can combine these changes with the .bazelrc changes @mcevoypeter made and remove copts specifications in the urbit and urbitdbg targets. Though that might complicate running standard builds. Not sure of the best way to handle this yet.

[mcevoypeter]

I think a well-rounded solution is to explicitly specify the optimization level via the copts attribute for all third party dependencies, remove the copts attribute from all of the targets that build our code (i.e. //...), and specify reasonable --copt defaults in .bazelrc. This will ensure our third party dependencies are always built with optimizations but allow us to specify the optimization level for any of our targets from the command line. Finally, it'll allow us to build a debug binary (with U3_CPU_DEBUG and U3_MEMORY_DEBUG defined and optimization set to -O0) by simply specifying --config=dbg (without having a separate debug target).

[barter-simsum]

@mcevoypeter I want that but slightly modified. I believe we should have a separate vere debug build target called something like urbitdbg. This target should NOT include defines like U3_MEMORY_DEBUG or U3_CPU_DEBUG which break loom/binary compatibility. It also shouldn't include optimization specifications since these should be configurable at the command line.

It should specify full debug symbols -g3 since the only reason to exclude that is to keep the binary size smaller which we don't care about when debugging (or even care about with the normal release binary).

It should also specify defines that don't break binary/loom compatibility but do add additional debug assertions, etc, e.g. the C3DBG macro in #164. It could also define U3_EVENT_TIME_DEBUG and maybe U3_MEMORY_LOG.

Maybe at some point we could also link in a profiler like gperftools to the debug target. Since we're producing static binaries, this is necessary -- unless you can easily produce a dynamically linked executable.

U3_MEMORY_DEBUG and U3_CPU_DEBUG should be wrapped like you did in a build:dbg configuration -- but maybe we call it something else. build:chk or build:wall. Those defines shouldn't be confused with a standard debug configuration.

I can add these changes in a bit.

[mcevoypeter]

What's the advantage to having a separate debug target as compared to doing all of what you mentioned via --copt. You can specify --copt on a per-user basis in .user.bazelrc, and that should be a flexible enough solution for people to produce any sort of debug-like build they want. We of course need documentation in INSTALL.md to explain how people can do that, but adding an entire separate target, largely a duplicate of //pkg/vere:urbit, seems unnecessary.

[barter-simsum]

@mcevoypeter Linking in a profiler would be a big reason. We don't want that in the release binary.

[barter-simsum]

I agree that essentially everything else could be implemented without a new target. Though I don't think U3_MEMORY_DEBUG and U3_CPU_DEBUG should be included by default. They should be explicitly enabled.

[mcevoypeter]

Linking in a profiler would be a big reason. We don't want that in the release binary.

Agreed, but unless we have plans on doing that now or in the near future, I think we should do everything via --copt, which should be easier to maintain instead of maintaining two mostly identical binary targets. If/when we do want to link in a profiler in the future, it'll be easy to add a debug target.

Though I don't think U3_MEMORY_DEBUG and U3_CPU_DEBUG should be included by default. They should be explicitly enabled.

Good call. I just pushed a commit that addresses this.

[mcevoypeter]

So now, the different cases mentioned by @philipcmonk look like :

• Optimized with CPU and memory debugging: bazel build --copt='-DU3_CPU_DEBUG' --copt='-DU3_MEMORY_DEBUG' :urbit
• Optimized without CPU and memory debugging: bazel build :urbit
• Unoptimized with CPU and memory debugging: bazel build --copt='-DU3_CPU_DEBUG' --copt='-DU3_MEMORY_DEBUG' --config=dbg :urbit
• Unoptimized without CPU and memory debugging: bazel build --config=dbg :urbit

Also, remember users can specify often-used command line options in .bazelrc, so in practice, bazel build :urbit will often be all a user needs to type to build assuming they've updated their .user.bazelrc accordingly.

[barter-simsum]

but unless we have plans on doing that now or in the near future, I think we should do everything via --copt

Agree. I have some work in progress to add gperftools, but it's not ready yet. Will be easy enough to make the debug config changes if those are added.

Approved.