Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why packed struct is so weird #13482

Closed
CGQAQ opened this issue Nov 8, 2022 · 3 comments
Closed

Why packed struct is so weird #13482

CGQAQ opened this issue Nov 8, 2022 · 3 comments
Labels
question No questions on the issue tracker, please.

Comments

@CGQAQ
Copy link

CGQAQ commented Nov 8, 2022

Reproduce repo

https://github.com/CGQAQ/zig-pnginfo/tree/reproduce

Zig Version

0.11.0-dev.86+b83e4d965

Steps to Reproduce and Observed Behavior

const std = @import("std");
const io = std.io;
const os = std.os;
const fs = std.fs;
const process = std.process;
const mem = std.mem;
const print = std.debug.print;

const png_CHUNK_IHDR = packed struct {
    length: u32 ,
    chunk_type: u32 ,
    data: png_IHDR,
    crc: u32 ,
};

const png_IHDR = packed struct {
    width: u32,
    height: u32,
    bit_depth: u8,
    color_type: u8,
    compression_method: u8,
    filter_method: u8,
    interlace_method: u8,
};

pub fn main() !void {
    const allocator = std.heap.page_allocator;

    const args = try process.argsAlloc(allocator);
    defer process.argsFree(allocator, args);

    if (args.len != 2) {
        _ = try io.getStdErr().write("Usage: zig-pnginfo <file.png>");
        return;
    }

    // resolve the path
    const path = try fs.realpathAlloc(allocator, args[1]);
    const file = try fs.openFileAbsolute(path, .{});
    defer file.close();

    const data = try file.readToEndAlloc(allocator, 102400000);

    if (mem.eql(u8, data[0..7], &[_]u8{137, 80, 78, 71, 13, 10, 26, 10})){
        _ = try io.getStdErr().write("Not a PNG file");
        return;
    }

    var offset: usize = 8;
    var iHDR = while(true) {
        // find the ihdr chunk
        var chunk: *png_CHUNK_IHDR = allocator.create(png_CHUNK_IHDR) catch unreachable;
        const chunk_size = @sizeOf(png_CHUNK_IHDR);
        @memcpy(@ptrCast([*]u8, &chunk), @ptrCast([*]const u8, &data[offset..]), chunk_size);

        print("chunk type: {d}\n", .{chunk.chunk_type});   // 1380206665

        chunk.length = @byteSwap(chunk.length);
        chunk.data.width = @byteSwap(chunk.data.width);
        chunk.data.height = @byteSwap(chunk.data.height);

        print("chunk type: {d}\n", .{chunk.chunk_type});  // 1200

        return;
        // if (chunk.chunk_type == 0x52444849){
        //     break chunk;
        // } else if(offset >= data.len){
        //     _ = try io.getStdErr().write("No IHDR chunk found");
        //     return;
        // } else {
        //     offset += 1;
        //     continue;
        // }
    };
    defer allocator.destroy(iHDR);


    std.debug.print("{}", .{iHDR});
}
PS D:\toyyy\zig-parser> zig build run -- apple.png
chunk type: 1380206665
chunk type: 1200

Expected Behavior

chunk.chunk_type should not be modified

Normal struct with alignment 1 run correctly

https://github.com/CGQAQ/zig-pnginfo/tree/main

const std = @import("std");
const io = std.io;
const os = std.os;
const fs = std.fs;
const process = std.process;
const mem = std.mem;
const print = std.debug.print;

const png_CHUNK_IHDR =  struct {
    length: u32 align(1),
    chunk_type: u32 align(1),
    data: png_IHDR align(1),
    crc: u32 align(1),
};

const png_IHDR = struct {
    width: u32 align(1),
    height: u32 align(1),
    bit_depth: u8,
    color_type: u8,
    compression_method: u8,
    filter_method: u8,
    interlace_method: u8,
};

pub fn main() !void {
    const allocator = std.heap.page_allocator;

    const args = try process.argsAlloc(allocator);
    defer process.argsFree(allocator, args);

    if (args.len != 2) {
        _ = try io.getStdErr().write("Usage: zig-pnginfo <file.png>");
        return;
    }

    // resolve the path
    const path = try fs.realpathAlloc(allocator, args[1]);
    const file = try fs.openFileAbsolute(path, .{});
    defer file.close();

    const data = try file.readToEndAlloc(allocator, 102400000);

    if (mem.eql(u8, data[0..7], &[_]u8{137, 80, 78, 71, 13, 10, 26, 10})){
        _ = try io.getStdErr().write("Not a PNG file");
        return;
    }

    var offset: usize = 8;
    var iHDR = while(true) {
        // find the ihdr chunk
        var chunk: *png_CHUNK_IHDR = allocator.create(png_CHUNK_IHDR) catch unreachable;
        const chunk_size = @sizeOf(png_CHUNK_IHDR);
        @memcpy(@ptrCast([*]u8, &chunk), @ptrCast([*]const u8, &data[offset..]), chunk_size);

        chunk.length = @byteSwap(chunk.length);
        chunk.data.width = @byteSwap(chunk.data.width);
        chunk.data.height = @byteSwap(chunk.data.height);

        if (chunk.chunk_type == 0x52444849){
            break chunk;
        } else if(offset >= data.len){
            _ = try io.getStdErr().write("No IHDR chunk found");
            return;
        } else {
            offset += 1;
            continue;
        }
    };
    defer allocator.destroy(iHDR);


    std.debug.print("{}", .{iHDR});
}
PS D:\toyyy\zig-parser> zig build run -- apple.png
main.png_CHUNK_IHDR{ .length = 13, .chunk_type = 1380206665, .data = main.png_IHDR{ .width = 1920, .height = 1200, .bit_depth = 8, .color_type = 6, .compression_method = 0, .filter_method = 0, .interlace_method = 0 }, .crc = 4132909082 }

in contrast, c works fine

#include <stdio.h>
#include <stdint.h>
#include <string.h>
#include <stdlib.h>
#include <assert.h>

#pragma pack(push, 1)
typedef struct png_IHDR {
	uint32_t width; // : u32,
	uint32_t height; // : u32,
	uint8_t bit_depth; // : u8,
	uint8_t color_type;// : u8,
	uint8_t compression_method;// : u8,
	uint8_t filter_method;// : u8,
	uint8_t interlace_method;// : u8,
} png_IHDR;
#pragma pack(pop)

#pragma pack(push, 1)
typedef struct png_CHUNK_IHDR {
	uint32_t length;// : u32,
		uint32_t chunk_type;// : u32,
		png_IHDR data;// : png_IHDR,
		uint32_t crc;// : u32,
} png_CHUNK_IHDR;
#pragma pack(pop)

void SwapBytes(void* pv, size_t n)
{
	assert(n > 0);

	char* p = pv;
	size_t lo, hi;
	for (lo = 0, hi = n - 1; hi > lo; lo++, hi--)
	{
		char tmp = p[lo];
		p[lo] = p[hi];
		p[hi] = tmp;
	}
}
#define SWAP(x) SwapBytes(&x, sizeof(x));

int main(int argc, char * argv[])
{
	FILE* png = NULL;
	uint8_t* buffer = malloc(1024);

	errno_t err = fopen_s(&png, "apple.png", "rb");
	if (err || png == NULL) {
		printf("error: %d", err);
		goto cleanup;
	}

	assert(strlen(buffer) > 1024);

	fread_s(buffer, 1024, 1, 1024, png);

	const uint8_t PNG_MAGIC[] = { 137, 80, 78, 71, 13, 10, 26, 10 };

	png_CHUNK_IHDR pngChunk;
	
	int cmp = memcmp(buffer, PNG_MAGIC, 8);

	if (!cmp) {
		errno_t err = memcpy_s(&pngChunk, sizeof(pngChunk), buffer + 8, sizeof(pngChunk));
		if (err) {
			printf("memcpy_s error: %d", err);
			goto cleanup;
		}

		printf("type %x\n", pngChunk.chunk_type);

		SWAP(pngChunk.length);
		SWAP(pngChunk.data.width);
		SWAP(pngChunk.data.height);


		printf("type %x", pngChunk.chunk_type);
	}


cleanup:
	free(buffer);
	fclose(png);
	return 0;
}
type 52444849
type 52444849

BTW, packed struct in Rust also can specify alignment, but packed struct in Zig only can specify backed integer

#![allow(unused)]
fn main() {
// Default representation, alignment lowered to 2.
#[repr(packed(2))]
struct PackedStruct {
    first: i16,
    second: i8,
    third: i32
}

// C representation, alignment raised to 8
#[repr(C, align(8))]
struct AlignedStruct {
    first: i16,
    second: i8,
    third: i32
}
@CGQAQ CGQAQ added the bug Observed behavior contradicts documented or intended behavior label Nov 8, 2022
@nektro
Copy link
Contributor

nektro commented Nov 8, 2022

packed structs in Zig do not align to packed structs in C. what you're likely looking for is extern struct setting align(1) on the fields. related: #13009 and #12852

extern struct is for guaranteeing layout, whereas packed struct is for making a struct that has a backing integer representation, similar to the way enums work

@CGQAQ
Copy link
Author

CGQAQ commented Nov 8, 2022

So, there's no way to specify whole struct alignment right now? thank you for answering me

@iacore
Copy link
Contributor

iacore commented Nov 8, 2022

Only pointer and slice/array has alignment

@Vexu Vexu closed this as completed Nov 8, 2022
@andrewrk andrewrk added question No questions on the issue tracker, please. and removed bug Observed behavior contradicts documented or intended behavior labels Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question No questions on the issue tracker, please.
Projects
None yet
Development

No branches or pull requests

5 participants