Skip to content

Commit

Permalink
cvtres: Exclude DLGINCLUDE resources from the coff object file
Browse files Browse the repository at this point in the history
cvtres.exe excludes DLGINCLUDE resources, but there's a bug where it will still write the name of the DLGINCLUDE resource (if it's a string) into the Resource Directory Strings section of the data (even though the string(s) in question are not actually referenced by anything, so they're entirely dead weight).

We avoid writing these orphaned strings, and therefore use a hacky workaround in the test code: we force the ID of generated DLGINCLUDE resources to always be an ordinal, and never a string.
  • Loading branch information
squeek502 committed Nov 25, 2024
1 parent 33575b8 commit 5448a57
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 3 deletions.
7 changes: 7 additions & 0 deletions src/cvtres.zig
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ pub const Resource = struct {
if (self.data.len != 0) return false;
return true;
}

pub fn isDlgInclude(resource: Resource) bool {
return resource.type_value == .ordinal and resource.type_value.ordinal == @intFromEnum(res.RT.DLGINCLUDE);
}
};

pub const ParsedResources = struct {
Expand All @@ -56,6 +60,7 @@ pub const ParsedResources = struct {

pub const ParseResOptions = struct {
skip_zero_data_resources: bool = true,
skip_dlginclude_resources: bool = true,
max_size: u64,
};

Expand Down Expand Up @@ -83,6 +88,8 @@ pub fn parseResInto(resources: *ParsedResources, reader: anytype, options: Parse
const resource_and_size = try parseResource(allocator, reader, bytes_remaining);
if (options.skip_zero_data_resources and resource_and_size.resource.data.len == 0) {
resource_and_size.resource.deinit(allocator);
} else if (options.skip_dlginclude_resources and resource_and_size.resource.isDlgInclude()) {
resource_and_size.resource.deinit(allocator);
} else {
errdefer resource_and_size.resource.deinit(allocator);
try resources.list.append(allocator, resource_and_size.resource);
Expand Down
40 changes: 39 additions & 1 deletion test/fuzzy_cvtres.zig
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const resinator = @import("resinator");
test "cvtres fuzz" {
const allocator = std.testing.allocator;
var random = std.Random.DefaultPrng.init(std.testing.random_seed);
var rand = random.random();
const rand = random.random();

var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
Expand Down Expand Up @@ -101,3 +101,41 @@ test "cvtres fuzz" {
};
}
}

test "all reserved types" {
const allocator = std.testing.allocator;
var random = std.Random.DefaultPrng.init(std.testing.random_seed);
const rand = random.random();

var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();

const tmp_path = try tmp.dir.realpathAlloc(allocator, ".");
defer allocator.free(tmp_path);

var data_buffer = std.ArrayList(u8).init(allocator);
defer data_buffer.deinit();

var res_buffer = std.ArrayList(u8).init(allocator);
defer res_buffer.deinit();

try utils.writePreface(res_buffer.writer());

// https://learn.microsoft.com/en-us/windows/win32/menurc/user-defined-resource
// > The numbers 1 through 255 are reserved for existing and future predefined resource types.
for (1..256) |predefined_type| {
_ = try utils.writeRandomValidResource(allocator, rand, res_buffer.writer(), .{
.set_type = .{ .ordinal = @intCast(predefined_type) },
.set_data = "foo",
});
}

// also write it to the top-level tmp dir for debugging
if (fuzzy_options.fuzzy_debug)
try std.fs.cwd().writeFile(.{ .sub_path = ".zig-cache/tmp/fuzzy_cvtres_reserved_types.res", .data = res_buffer.items });

try utils.expectSameCvtResOutput(allocator, res_buffer.items, .{
.cwd = tmp.dir,
.cwd_path = tmp_path,
});
}
15 changes: 13 additions & 2 deletions test/utils.zig
Original file line number Diff line number Diff line change
Expand Up @@ -852,11 +852,22 @@ pub const RandomResourceOptions = struct {
/// Returns the data size of the resource that was written
pub fn writeRandomValidResource(allocator: Allocator, rand: std.Random, writer: anytype, options: RandomResourceOptions) !u32 {
const data_size: u32 = if (options.set_data) |data| @intCast(data.len) else rand.uintAtMostBiased(u32, 150);
const name_value = options.set_name orelse try getRandomNameOrOrdinal(allocator, rand, 32);
defer if (options.set_name == null) name_value.deinit(allocator);
const type_value = options.set_type orelse try getRandomNameOrOrdinal(allocator, rand, 32);
defer if (options.set_type == null) type_value.deinit(allocator);

const name_value = name_value: {
// Avoid a cvtres.exe miscompilation where DLGINCLUDE resources are excluded, but their name
// value is written into the strings section anyway (even though it is not referenced by anything).
// This is a hacky approach, but we just force the ID to be an ordinal whenever the type is DLGINCLUDE.
if (type_value == .ordinal and type_value.ordinal == @intFromEnum(resinator.res.RT.DLGINCLUDE)) {
if (options.set_name != null and options.set_name.? == .ordinal)
break :name_value options.set_name.?;
break :name_value resinator.res.NameOrOrdinal{ .ordinal = rand.int(u16) };
}
break :name_value options.set_name orelse try getRandomNameOrOrdinal(allocator, rand, 32);
};
defer if (options.set_name == null) name_value.deinit(allocator);

const header = resinator.compile.Compiler.ResourceHeader{
.name_value = name_value,
.type_value = type_value,
Expand Down

0 comments on commit 5448a57

Please # to comment.