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
- Touch up `ds` documentation as per rednex/rgbds#350 — committed to ISSOtm/rgbds by ISSOtm 5 years ago
- Touch up `ds` documentation as per rednex/rgbds#350 — committed to ISSOtm/rgbds by ISSOtm 5 years ago
- Touch up `ds` documentation as per rednex/rgbds#350 — committed to ISSOtm/rgbds by ISSOtm 5 years ago
- Touch up `ds` documentation as per rednex#350 — committed to ISSOtm/rgbds by ISSOtm 5 years ago
- Touch up `ds` documentation as per rednex#350 — committed to ISSOtm/rgbds by ISSOtm 5 years ago
- Touch up `ds` documentation as per rednex#350 — committed to ISSOtm/rgbds by ISSOtm 5 years ago
- Add `ds cnt, byte` syntax As suggested by https://github.com/rednex/rgbds/issues/350#issuecomment-498030458 The order `count` then `byte` was decided after some discussion: - First argument consisten... — committed to gbdev/rgbds by ISSOtm 4 years ago
Here is a second draft:
@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 forrst $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 likeDBREP
andDWREP
. These would take one length argument, and one or more data arguments of size byte or word. For example……would output the same data as…
So it would be a shorthand for a
DB
/DW
in aREPT
. 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 likeThis 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 theDS
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 howDS
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:
-p
flag affects theDS
directive.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 whatds
already does, and what pino is describing, and.fill
that works more like what nitro describes.