From ad42376d5e7b0188b56203213715ce07d42d8f26 Mon Sep 17 00:00:00 2001 From: Alex Lin Date: Thu, 1 Jun 2023 13:20:14 -0500 Subject: [PATCH] Variable Server byteswapping crashes the sim #1513 (#1514) * Variable Server byteswapping crashes the sim #1513 The code to byteswap a variable server buffer has a comment saying there is a bug. The comment is correct. The original code would swap the parameter and anything else that followed that parameter in the structure. Crashes everywhere. Created a new routine that byteswaps a single parameter. Strangely we didn't have such a routine until now. Did some testing of doubles, floats, ints, shorts, and chars and all were swapped correctly. * Variable Server byteswapping crashes the sim #1513 enabling binary byteswap test. --- include/trick/tc_proto.h | 1 + .../models/test_client/test_client.cpp | 2 +- .../VariableServerThread_write_data.cpp | 3 +- .../trick_utils/comm/src/trick_bswap_buffer.c | 2 +- .../comm/src/trick_bswap_single_parameter.c | 117 ++++++++++++++++++ 5 files changed, 121 insertions(+), 4 deletions(-) create mode 100644 trick_source/trick_utils/comm/src/trick_bswap_single_parameter.c diff --git a/include/trick/tc_proto.h b/include/trick/tc_proto.h index 60abf039..0abf0e79 100644 --- a/include/trick/tc_proto.h +++ b/include/trick/tc_proto.h @@ -112,6 +112,7 @@ int tc_error(TCDevice * device, int on_off); int tc_dev_copy(TCDevice * dest, TCDevice * src); void *trick_bswap_buffer(void *out, void *in, ATTRIBUTES * attr, int tofrom) ; +void *trick_bswap_single_parameter(void *out, void *in, ATTRIBUTES * attr, int tofrom) ; #ifdef __cplusplus } #endif diff --git a/test/SIM_test_varserv/models/test_client/test_client.cpp b/test/SIM_test_varserv/models/test_client/test_client.cpp index 4c714be3..070606ed 100644 --- a/test/SIM_test_varserv/models/test_client/test_client.cpp +++ b/test/SIM_test_varserv/models/test_client/test_client.cpp @@ -1580,7 +1580,7 @@ TEST_F (VariableServerTest, Binary) { EXPECT_EQ(variable_noname.getValue(), true); } -TEST_F (VariableServerTest, DISABLED_BinaryByteswap) { +TEST_F (VariableServerTest, BinaryByteswap) { std::vector reply; socket << "trick.var_binary()\ntrick.var_byteswap(False)\ntrick.var_add(\"vsx.vst.f\")\ntrick.var_add(\"vsx.vst.j\")\n"; diff --git a/trick_source/sim_services/VariableServer/VariableServerThread_write_data.cpp b/trick_source/sim_services/VariableServer/VariableServerThread_write_data.cpp index 651e1462..17709b3d 100644 --- a/trick_source/sim_services/VariableServer/VariableServerThread_write_data.cpp +++ b/trick_source/sim_services/VariableServer/VariableServerThread_write_data.cpp @@ -90,8 +90,7 @@ int Trick::VariableServerThread::write_binary_data( int Start, char *buf1, const memcpy(&buf1[offset] , &swap_int , sizeof(size)) ; offset += sizeof(size) ; - /* TODO: There is a bug here, this call will want to swap the entire buffer, we may not have the whole buffer */ - trick_bswap_buffer(&buf1[offset], address, given_vars[i]->ref->attr, 1); + trick_bswap_single_parameter(&buf1[offset], address, given_vars[i]->ref->attr, 1); offset += size ; } else { diff --git a/trick_source/trick_utils/comm/src/trick_bswap_buffer.c b/trick_source/trick_utils/comm/src/trick_bswap_buffer.c index 0591d8a0..beb54992 100644 --- a/trick_source/trick_utils/comm/src/trick_bswap_buffer.c +++ b/trick_source/trick_utils/comm/src/trick_bswap_buffer.c @@ -29,7 +29,7 @@ void *trick_bswap_buffer(void *out, void *in, ATTRIBUTES * attr, int tofrom) i = 0; TRICK_GET_BYTE_ORDER(local_order); - while (attr[i].name[0] != '\0') { + while ((attr[i].name != NULL) && (attr[i].name[0] != '\0')) { num = 1; for (j = 0; j < attr[i].num_index; j++) { diff --git a/trick_source/trick_utils/comm/src/trick_bswap_single_parameter.c b/trick_source/trick_utils/comm/src/trick_bswap_single_parameter.c new file mode 100644 index 00000000..8f1b88a2 --- /dev/null +++ b/trick_source/trick_utils/comm/src/trick_bswap_single_parameter.c @@ -0,0 +1,117 @@ + +#include + +#include "trick/attributes.h" +#include "trick/parameter_types.h" +#include "trick/trick_byteswap.h" +#include "trick/tc_proto.h" + +/* + * tofrom: 1 = to. + * Use 1 (to) before writing data. Converts to the other endian + * + * tofrom: 0 = from. + * Use 0 (from) after reading data. Converts from the other endian + */ + +void *trick_bswap_single_parameter(void *out, void *in, ATTRIBUTES * attr, int tofrom) +{ + + int i, j; + unsigned short ts; + unsigned int ti; + double td; + int num; + unsigned int mask; + int shift; + int local_order; + + i = 0; + TRICK_GET_BYTE_ORDER(local_order); + + if ((attr[i].name != NULL) && (attr[i].name[0] != '\0')) { + + num = 1; + for (j = 0; j < attr[i].num_index; j++) { + num *= attr[i].index[j].size; + } + + if (attr[i].type == TRICK_STRUCTURED) { + + for (j = 0; j < num; j++) { + trick_bswap_buffer((char *) ((char *) out + attr[i].offset + j * attr[i].size), + (char *) ((char *) in + attr[i].offset + j * attr[i].size), + (ATTRIBUTES *) attr[i].attr, tofrom); + } + + } else if (attr[i].type == TRICK_BITFIELD || attr[i].type == TRICK_UNSIGNED_BITFIELD) { + + ti = *(int *) ((char *) in ); + + /* swap this word if this is incoming data */ + if (tofrom == 0) { + ti = trick_byteswap_int((int) ti); + } + + if ((local_order == TRICK_LITTLE_ENDIAN && tofrom == 1) || (local_order == TRICK_BIG_ENDIAN && tofrom == 0)) { + + /* Make a mask of "size" bits and shift it to the correct bit position */ + mask = + (~(0xffffffff << attr[i].index[0].size)) << (32 - attr[i].index[0].start - attr[i].index[0].size); + ti &= mask; + + /* Calculate shift to new location */ + shift = (2 * attr[i].index[0].start) + attr[i].index[0].size - 32; + } else { + + /* Make a mask of "size" bits and shift it to the correct bit position */ + mask = + (~(0xffffffff >> attr[i].index[0].size)) >> (32 - attr[i].index[0].start - attr[i].index[0].size); + ti &= mask; + + /* Calculate shift to new location */ + shift = 32 - (2 * attr[i].index[0].start) - attr[i].index[0].size; + } + if (shift > 0) { + ti = ti << shift; + } else { + ti = ti >> -shift; + } + + /* Mask in the new bits */ + *(int *) ((char *) out ) = ti; + + } else { + + switch (attr[i].size) { + + case 1: + for (j = 0; j < num; j++) { + *(unsigned char *) ((char *) out + j) = *(unsigned char *) ((char *) in + j); + } + break; + case 2: + for (j = 0; j < num; j++) { + ts = *(unsigned short *) ((char *) in + j * 2); + *(unsigned short *) ((char *) out + j * 2) = trick_byteswap_short((short) ts); + } + break; + case 4: + for (j = 0; j < num; j++) { + ti = *(unsigned int *) ((char *) in + j * 4); + *(unsigned int *) ((char *) out + j * 4) = trick_byteswap_int((int) ti); + } + break; + case 8: + for (j = 0; j < num; j++) { + td = *(double *) ((char *) in + j * 8); + *(double *) ((char *) out + j * 8) = trick_byteswap_double(td); + } + break; + } + } + i++; + } + + return ((void *) out); +}