From: Bernhard W. <bw...@su...> - 2007-08-24 15:50:15
|
Hello, I'd like to get opinions about this one. I had a bug report about this, so it's a "real" problem. ---- The index file is architecture independent in two ways: - size of the types (a pointer is saved to disk) - endianess So if the index file was created on one machine, you have to delete the index file in order to be able to use cross-debugging again. Otherwise, lcrash crashes. There's no field in the index header that can be used to get the machine type where the index file has been created which would be necessary to detect that problem in a clean way. To add that field, it would be also necessary to change the index format, so I thought it's better to implement an architecture independent format. The patch doesn't remove the capability to read the old format, it only removes the capability to *write* the old format. It changes the magic number of the dump, because, if you only change the version, old versions of lcrash still think they can read the new format: if (dump_index.version_number < DUMP_INDEX_VERSION) /* cannot read */ which means if (dump_index.version_number >= DUMP_INDEX_VERSION) /* can read */ However, this is wrong because the new format must be incompatible. It doesn't define an endian format for the disk index, it just saves the endianess that was used to create the index. This is for performance reasons: The most likely case is that you use the same machine type for saving and loading, and so no byte swapping is necessary in the normal case. Signed-off-by: Bernhard Walle <bw...@su...> --- lib/libklib/include/kl_cmp.h | 26 +++- lib/libklib/kl_cmp.c | 249 +++++++++++++++++++++++++++++++++++-------- 2 files changed, 230 insertions(+), 45 deletions(-) --- a/lib/libklib/include/kl_cmp.h +++ b/lib/libklib/include/kl_cmp.h @@ -21,9 +21,11 @@ #ifndef __KL_CMP_H #define __KL_CMP_H -#define DUMP_INDEX_MAGIC 0xdeadbeef -#define DUMP_INDEX_VERSION 31900 -#define NUM_BUCKETS 65535 +#define DUMP_INDEX_MAGIC 0xdeadbeef +#define DUMP_INDEX_MAGIC_AI 0xdeadbafe +#define DUMP_INDEX_VERSION 31900 +#define DUMP_INDEX_VERSION_AI 10000 +#define NUM_BUCKETS 65535 /* * Definitions for compressed cached reads. I've recently lowered @@ -77,8 +79,26 @@ typedef struct dump_index_s { struct {uint64_t tv_sec; uint64_t tv_usec; } timebuf; /* the time of the dump */ + uint32_t machine_flags; /* this is only for DUMP_INDEX_VERSION_AI)! */ } __attribute__((packed)) dump_index_t; +/* + * while above is the classical format (DUMP_INDEX_VERSION), this + * has the problem that you need to load the dump on the same machine + * type again and again ... otherwise lcrash crashes + * + * This new format is machine independent (endian type is in the index, so + * swapping is only necessary if you change the machine) + */ +typedef struct _ptableindex_ai { + kl_dump_page_t dir; /* already architecture independent */ + kaddr_t addr; + kaddr_t coreaddr; + uint32_t hash; +} __attribute__((packed)) ptableindex_ai; + + + /* Compression function */ typedef int (*kl_compress_fn_t)(const unsigned char *old, uint32_t old_size, unsigned char *new, uint32_t size); --- a/lib/libklib/kl_cmp.c +++ b/lib/libklib/kl_cmp.c @@ -59,6 +59,7 @@ static int __cmpphash(kaddr_t addr); static int __cmpcheckpageheader(int fd, kl_dump_page_t *dp); static int __cmppindexcreate(int fd, char *indexname, int flags); static int __cmpcheckheader(int fd); +void cmp_convert_classic_to_ai(ptableindex *input, ptableindex_ai *output); /* @@ -710,10 +711,11 @@ __cmpsaveindex(char *filename, int flags } /* create the dump index */ - dump_index.magic_number = DUMP_INDEX_MAGIC; - dump_index.version_number = DUMP_INDEX_VERSION; + dump_index.magic_number = DUMP_INDEX_MAGIC_AI; + dump_index.version_number = DUMP_INDEX_VERSION_AI; dump_index.timebuf.tv_sec = dh->time.tv_sec; dump_index.timebuf.tv_usec = dh->time.tv_usec; + dump_index.machine_flags = KLP->host->byteorder; /* write the index header */ if (write(ifd, (const void *)&dump_index, @@ -729,10 +731,13 @@ __cmpsaveindex(char *filename, int flags } for (i = 0; i < NUM_BUCKETS; i++) { + ptableindex_ai tmp_ai; + tmpptr = page_index[i]; while (tmpptr) { - if (write(ifd, (const void *)tmpptr, - sizeof(ptableindex)) != sizeof(ptableindex)) { + cmp_convert_classic_to_ai(tmpptr, &tmp_ai); + if (write(ifd, (const void *)&tmp_ai, + sizeof(ptableindex_ai)) != sizeof(ptableindex_ai)) { fprintf(KL_ERRORFP, "\n__cmpsaveindex(): save of " "index \"%s\" failed!\n", @@ -876,6 +881,173 @@ __cmpcleanindex(void) } } +static inline uint64_t dump2host64(uint32_t machine_flags, uint64_t input) +{ + if ((machine_flags & (KL_LITTLE_ENDIAN | KL_BIG_ENDIAN)) != KLP->host->byteorder) + return kl_get_swap_uint64(&input); + else + return input; +} + +static inline uint32_t dump2host32(uint32_t machine_flags, uint32_t input) +{ + if ((machine_flags & (KL_LITTLE_ENDIAN | KL_BIG_ENDIAN)) != KLP->host->byteorder) + return kl_get_swap_uint32(&input); + else + return input; +} + +/* + * converts between machine dependent representation and machine independent + * representation, also swaps endianess if necessary (read from machine_flags) + */ +void cmp_convert_ai_to_classic(uint32_t machine_flags, + ptableindex_ai *input, + ptableindex *output) +{ + if (!input || !output) + return; + +#if DUMP_DEBUG >= 6 + output->dir.byte_offset = dump2host64(machine_flags, input->dir.byte_offset); + output->dir.page_index = dump2host64(machine_flags, input->dir.page_index); +#endif + output->dir.address = dump2host64(machine_flags, input->dir.address); + output->dir.size = dump2host32(machine_flags, input->dir.size); + output->dir.flags = dump2host32(machine_flags, input->dir.flags); + output->addr = dump2host64(machine_flags, input->addr); + output->coreaddr = dump2host64(machine_flags, input->coreaddr); + output->hash = dump2host64(machine_flags, input->hash); + output->next = NULL; +} + + +/* + * Converts between machine independent representation and machine dependent + * representation. Because the file is always written in host byte order, + * there's no byte swapping necessary. + */ +void cmp_convert_classic_to_ai(ptableindex *input, ptableindex_ai *output) +{ + if (!input || !output) + return; + + output->dir = input->dir; + output->addr = input->addr; + output->coreaddr = input->coreaddr; + output->hash = input->hash; +} + +/* + * loads the classic format, i.e. architecture + */ +static int __cmploadindex_classic(dump_index_t *index, int ifd) +{ + int result; + ptableindex *ptr1, *ptr2; + + /* okee, we're good to go -- try to load the index */ + result = 1; + while (result) { + ptr1 = (ptableindex *)malloc(sizeof(ptableindex)); + result = read(ifd, (void *)ptr1, sizeof(ptableindex)); + if (!result) { + if (cmp_debug) { + fprintf(KL_ERRORFP, "complete.\n"); + } + (void)close(ifd); + return (1); + } else if (result < 0) { + if (cmp_debug) { + fprintf(KL_ERRORFP, "__cmploadindex(): " + "index file read() failed!\n"); + } + (void)close(ifd); + return (-1); + } else if (result != sizeof(ptableindex)) { + if (cmp_debug) { + fprintf(KL_ERRORFP, "__cmploadindex(): " + "index file read() short!\n"); + } + (void)close(ifd); + return (-1); + } + ptr1->next = (ptableindex *)NULL; + ptr2 = page_index[ptr1->hash]; + page_index[ptr1->hash] = ptr1; + page_index[ptr1->hash]->next = ptr2; + } + + if (cmp_debug) { + fprintf(KL_ERRORFP, + "__cmploadindex(): read() past end of index file!\n"); + } + (void)close(ifd); + return (-1); +} + +/* + * loads the new format, i.e. architecture independent + */ +static int __cmploadindex_ai(dump_index_t *index, int ifd) +{ + int result; + ptableindex *ptr1, *ptr2; + + /* okee, we're good to go -- try to load the index */ + result = 1; + while (result) { + ptableindex_ai input; + + result = read(ifd, &input, sizeof(ptableindex_ai)); + if (!result) { + if (cmp_debug) { + fprintf(KL_ERRORFP, "complete.\n"); + } + close(ifd); + return (1); + } else if (result < 0) { + if (cmp_debug) { + fprintf(KL_ERRORFP, "__cmploadindex_ai(): " + "index file read() failed!\n"); + } + close(ifd); + return (-1); + } else if (result != sizeof(ptableindex_ai)) { + if (cmp_debug) { + fprintf(KL_ERRORFP, "__cmploadindex_ai(): " + "index file read() short!\n"); + } + close(ifd); + return (-1); + } + + ptr1 = malloc(sizeof(ptableindex)); + cmp_convert_ai_to_classic(index->machine_flags, &input, ptr1); + if (!ptr1) { + if (cmp_debug) { + fprintf(KL_ERRORFP, "__cmploadindex_ai(): " + "Failed to convert between machine independent" + "index file and machinde dependent structures"); + } + close(ifd); + return (-1); + } + + ptr1->next = (ptableindex *)NULL; + ptr2 = page_index[ptr1->hash]; + page_index[ptr1->hash] = ptr1; + page_index[ptr1->hash]->next = ptr2; + } + + if (cmp_debug) { + fprintf(KL_ERRORFP, + "__cmploadindex(): read() past end of index file!\n"); + } + (void)close(ifd); + return (-1); +} + /* * __cmploadindex() * @@ -886,9 +1058,13 @@ __cmpcleanindex(void) static int __cmploadindex(char *indexname) { - int ifd, result; - ptableindex *ptr1, *ptr2; + static int versions[] = { + DUMP_INDEX_VERSION, + DUMP_INDEX_VERSION_AI + }; + int ifd; dump_index_t dump_index; + int ai_format; /* if we aren't given an index name, bail */ if ((!indexname) || (indexname[0] == '\0')) { @@ -921,7 +1097,8 @@ __cmploadindex(char *indexname) } /* make sure the dump index magic is valid */ - if (dump_index.magic_number != DUMP_INDEX_MAGIC) { + if (dump_index.magic_number != DUMP_INDEX_MAGIC && + dump_index.magic_number != DUMP_INDEX_MAGIC_AI) { if (cmp_debug) { fprintf(KL_ERRORFP, "__cmploadindex(): " "DUMP_INDEX_MAGIC in index file is wrong!\n"); @@ -929,9 +1106,10 @@ __cmploadindex(char *indexname) (void)close(ifd); return (-1); } + ai_format = dump_index.magic_number == DUMP_INDEX_MAGIC_AI; /* make sure the version number is up to date */ - if (dump_index.version_number < DUMP_INDEX_VERSION) { + if (dump_index.version_number < versions[ai_format]) { if (cmp_debug) { fprintf(KL_ERRORFP, "__cmploadindex(): " "DUMP_INDEX_VERSION in index file " @@ -967,44 +1145,31 @@ __cmploadindex(char *indexname) "index \"%s\" ... ", indexname); } - /* okee, we're good to go -- try to load the index */ - result = 1; - while (result) { - ptr1 = (ptableindex *)malloc(sizeof(ptableindex)); - result = read(ifd, (void *)ptr1, sizeof(ptableindex)); - if (!result) { - if (cmp_debug) { - fprintf(KL_ERRORFP, "complete.\n"); - } - (void)close(ifd); - return (1); - } else if (result < 0) { - if (cmp_debug) { - fprintf(KL_ERRORFP, "__cmploadindex(): " - "index file read() failed!\n"); - } - (void)close(ifd); - return (-1); - } else if (result != sizeof(ptableindex)) { - if (cmp_debug) { - fprintf(KL_ERRORFP, "__cmploadindex(): " - "index file read() short!\n"); - } - (void)close(ifd); + if (ai_format) { + if (cmp_debug) + fprintf(KL_ERRORFP, "__cmploadindex(): " + "Using architecture independent index file format!\n"); + + return __cmploadindex_ai(&dump_index, ifd); + } else { + off_t ret; + + if (cmp_debug) + fprintf(KL_ERRORFP, "__cmploadindex(): " + "Using architecture classic index file format!\n"); + + /* we read too much input because the actual + * sizeof(dump_index_t) was too much */ + ret = lseek(ifd, -(int)sizeof(dump_index.machine_flags), SEEK_CUR); + if (ret == (off_t)-1) { + fprintf(KL_ERRORFP, "__cmploadindex(): " + "lseek() failed for index file\n"); + close(ifd); return (-1); } - ptr1->next = (ptableindex *)NULL; - ptr2 = page_index[ptr1->hash]; - page_index[ptr1->hash] = ptr1; - page_index[ptr1->hash]->next = ptr2; - } - if (cmp_debug) { - fprintf(KL_ERRORFP, - "__cmploadindex(): read() past end of index file!\n"); + return __cmploadindex_classic(&dump_index, ifd); } - (void)close(ifd); - return (-1); } /* |