rgbds: `ds` content is documented as undefined in ROM sections

The content fillled by the ds directive in ROM sections is documented as undefined yet is is very well defined.

Can a programmer rely on the ds areas being filled with the value to -p or not?

Also, it isn’t very clear from the command-line usage that this value is used for the ds directive. (padding an image is only really relevant to rgblink and rgbfix)

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Comments: 38 (38 by maintainers)

Commits related to this issue

Most upvoted comments

Here is a second draft:

   Declaring variables in a RAM section
     DS allocates a number of empty bytes.  This is the preferred method of
     allocating space in a RAM section.  You can also use DB, DW and DL
     without any arguments instead (see Defining constant data below).

           DS 42 ; Allocates 42 bytes

     Empty space in RAM sections will not be initialized. In ROM sections, it
     will be filled with the value passed to the -p command-line option,
     except when using overlays with -O.

@aaaaaa123456789 Just to be clear, what I mentioned in the previous comment was a suggestion on how to reimplement your use case using existing features. It was not a new feature suggestion. The -D flag for defining a symbol already exists since however many versions back. And the rest is just a standard syntax for repeating a constant byte x times. Which again you could using existing functionality and not many more bytes of code.

My opposition to using DS this way is also somewhat philosophical. Reusing a directive normally used for data in RAM where it by its very nature is undefined, is a kludge, and passing the fill value as a switch is an afterthought that just happened to be useful for your use case. It doesn’t really fit in with how the assembler works overall. I’d much rather that you’d solve this using more generic building blocks, like my suggestion above.

Another “philosophically consistent” way of implementing it would be to let the assembler leave bytes defined by DS undefined and let the linker fill them in with its fill value. But this would potentially break existing code since a common use is apparently to use this to fill in the header with zeroes, whereas you’d often want the linker to fill with $FF both for preserving the life of the flash chips in flash cartridges, and for catching potential out of bounds execution since $FF is the opcode for rst $38 which in turn can be used as a crash handler.

@pinobatch I do like the idea, but I’d implement it rather differently because of my aforementioned philosophical opposition to this use of the DS which I’d rather see deprecated in the long term, in particular when used without a data argument. Namely, I’d have two directives called something like DBREP and DWREP. These would take one length argument, and one or more data arguments of size byte or word. For example…

    DBREP 4,0,1,2,3

…would output the same data as…

    DB 0,1,2,3,0,1,2,3,0,1,2,3,0,1,2,3

So it would be a shorthand for a DB/DW in a REPT. But in its simplest use, you’d give it one length argument and one data argument.

Warning: rant incoming!

Daily reminder that undefined doesn’t mean random. It just means that you’re not allowed to rely on the value, even if it always happens to be initialized to a certain value in a given implementation. And likewise, it means that when you define memory using DS you forfeit that the right to rely on the value being defined.

My claim, then, would be that it is strictly speaking a user error to use DS for defining constant data, however convenient and elegant it looks. The better way would be something like

REPT 76
    DB 0
ENDR

This makes sense because traditionally in assemblers, DS (define storage) has usually only been used for static allocation in RAM, which by its very nature is undefined. Especially on GB where RAM is uninitialized on startup. This is also reflected in the title of the document, “Declaring variables in a RAM section” which has gone unedited since the Carsten Sorensen days, apart from replacing BSS with RAM.

However, the documentation doesn’t lie, or at least it didn’t when it was written. Here’s a blast form the past. The original versions of RGBDS, as released by Carsten himself, didn’t clear the memory buffers before writing data to them. This was mostly not a problem since in practice they would mostly be zeroed. With the exception of the first 8 or so byte of the ROM, which would contain some junk data, probably because that buffer was realloc’d from a previous use. So I suspect that what the fill flag would actually do in previous versions of the assembler is to pre-clear the buffer, rather than use that as a defined value in the skip function. This issue showed up for me in the linker, but also applied to the assembler. I wasn’t happy with it at the time, but I just used added -zff to both tools (which was the fill flag for those versions) and went on with my life. Also, at the time, RGBFIX wouldn’t complain about non-zero values so this wouldn’t have been an issue to begin with.

Allowing DS to be used for static ROM allocation is a bit of a kludge, and as far as I know, allowing the DS directive for ROM data is the only reason the assembler has a fill flag to begin with. The assembler shouldn’t really need one. (Only the linker should, to fill in unallocated space.) If I redesigned RGBDS from the ground up I would simply remove this feature. But now it’s a well established part of RGBDS so it needs to remain for backward compatibility.

I would strongly oppose any suggestions to randomize the fill value for DS, just to conform with a documentation originally written to describe how DS works when allocating RAM. That would be sheer lunacy. An assembler, given the same input twice, should generally produce the same result, unless you do something that very explicitly differs between runs. (Like a current date function or an explicit randomness function.)

I would instead propose the following:

  1. Document how the -p flag affects the DS directive.
  2. As far as the implementation goes, either change nothing, or warn/error if DS is used without specifying the -p flag.

Also, as the final part of the rant, I want to ask @aaaaaa123456789 exactly how that free space calculator would work. How could a tool rely on any continuous area of a certain value that’s not at the end of a file/bank being empty? How would the tool know that this data was truly empty and not part of a data structure that just happens to be filled with a bunch of zeroes? It seems like such a tool would give wildly inaccurate results for many files. Furthermore, why use a binary analysis tool for output from RGBDS, when the linker can output a perfectly usable map file using the flag -m, which tells you in no uncertain terms which and how many bytes are allocated or not?

Imo, just document its current behavior properly. It’s not exactly surprising or unintended behavior like #190 was, and people kind of expect it to work this way.

Heck, if anything, I’m just going to point to existing behavior of binutils, with its .skip and .org directives that do essentially what ds already does, and what pino is describing, and .fill that works more like what nitro describes.